Some improvements for Arduino Binding, new value filter classes added - #2588
Some improvements for Arduino Binding, new value filter classes added#2588pgrawehr wants to merge 8 commits into
Conversation
dotnet-policy-service
Bot
added
the
area-device-bindings
| _holdingState = ButtonHoldingState.Completed; | ||
| Holding?.Invoke(this, new ButtonHoldingEventArgs { HoldingState = ButtonHoldingState.Completed }); | ||
| } | ||
| else |
There was a problem hiding this comment.
Local agent review:
ButtonBase.Press semantics changed silently (src/devices/Button/ButtonBase.cs:132-142). Press used to fire on every release; the PR moves it into an else of the hold-check, so consumers subscribing only to Press stop getting events after a long press. Public event on a widely used binding, no doc/test/PR-notes for the change.
| /// <typeparam name="TResult">The result of the filtering. Can be a single value (e.g. for an averaging filter) | ||
| /// or a list of TSource</typeparam> | ||
| /// <remarks>This class is thread safe. Concrete implementations should maintain that.</remarks> | ||
| public abstract class ValueFilter<TSourceElement, TResult> |
There was a problem hiding this comment.
Local agent comment:
ValueFilter<T> thread-safety claim is overstated (ValueFilter.cs:38-45, 55-71, 95-104). AddElement/Clear skip _readLock; CurrentValue releases and re-takes the lock between RemoveOldElements and ToArray, so a racing Clear can drop elements the peek didn't observe. Either tighten the locking or drop the "thread safe" remark.
| /// </summary> | ||
| /// <param name="values">The values to average over</param> | ||
| /// <returns>The return value</returns> | ||
| public static T? AverageFilter(List<T> values) |
There was a problem hiding this comment.
Local agent comment:
TimeSliceFilter<T>.AverageFilter assumes default(T) is additive identity (TimeSliceFilter.cs:132-148). Safe for BCL primitives, wrong for UnitsNet-style struct wrappers. Seed the sum from values[0] like the sibling Min/Max filters.
2 participants
Including some new features to the test application
Microsoft Reviewers: Open in CodeFlow