-
Notifications
You must be signed in to change notification settings - Fork 607
Various improvements to Nmea and AIS parsers #2430
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
public Angle? MagneticVariation | ||
{ | ||
get; | ||
private set; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why no setter? (others do have it?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because that's an output from this class, not an input like most other properties.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
SendCommand(seq); | ||
} | ||
|
||
public TimeSpan QueryAnalogInputSamplingInterval() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider
public TimeSpan QueryAnalogInputSamplingInterval() | |
public TimeSpan AnalogInputSamplingInterval => ... |
or renaming Query
to Get
to make it sound simpler
/// This is useful to e.g. trigger an error only after an input value | ||
/// is high for at least a certain time. | ||
/// </summary> | ||
public class HysteresisFilter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about the word Filter
in the name but I cannot think of anything better... maybe HysteresisFunction?
NMEA and AIS features verified in real-world scenarios.