Skip to content

Conversation

@SpangeJ
Copy link

@SpangeJ SpangeJ commented Jan 31, 2025

Write parsers and tests for new NMEA messages:

  • HDT
  • DPT
  • PTNL,GGK
  • XDR
  • AML,SVM

Joachim Spange added 8 commits January 28, 2025 11:19
Copy link
Owner

@dotMorten dotMorten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dpt, Xdr and Hdt (the latter being deprecated) are the only ones part of the NMEA spec.
The others seem to rely on an odd Trimble pattern that I think would require some adjustments to the underlying library for how messages like that are being identified since they rely on multiple columns. I'm not really in favor of hardcoding those into the message parser, but would prefer you first open an discussing the need for parsing messages like that, and then probably keep these messages locally as message extensions in the application and not in the NMEA library itself.

/// </para>
/// </remarks>
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly", MessageId = "HeHdt")]
[NmeaMessageType("--HDT")]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the NMEA spec HDT is a deprecated message

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you which me to revert it?
I can just create it locally in my project if you think it should not be part of this library.

[TestMethod]
public void TestDPT()
{
string input = "$--DPT,149.5,000.5,1000.0*7F";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really how a real world message looks with the -- prefix and no talker-id?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, my data looks like that.

@SpangeJ
Copy link
Author

SpangeJ commented Feb 3, 2025

Dpt, Xdr and Hdt (the latter being deprecated) are the only ones part of the NMEA spec. The others seem to rely on an odd Trimble pattern that I think would require some adjustments to the underlying library for how messages like that are being identified since they rely on multiple columns. I'm not really in favor of hardcoding those into the message parser, but would prefer you first open an discussing the need for parsing messages like that, and then probably keep these messages locally as message extensions in the application and not in the NMEA library itself.

I agree with your reasoning. PTNL,GGK and AML,SVM are different since they rely on the two first part of the message in order to identify what kind of message it is.

I will remove them from this PR.
I should solve the Trimble pattern properly in a separate PR. I'll open a discussion prior to that PR.

@SpangeJ SpangeJ requested a review from dotMorten February 5, 2025 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants