Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[NMEA2K]: Add general NMEA2000 message interface, fix FP issues #288

Merged
merged 11 commits into from
Oct 28, 2023
Merged

[NMEA2K]: Add general NMEA2000 message interface, fix FP issues #288

merged 11 commits into from
Oct 28, 2023

Conversation

ad3154
Copy link
Member

@ad3154 ad3154 commented Jul 14, 2023 •
edited
Loading

What's New:

  • Added message definitions and objects for the following NMEA2000 messages that are commonly found on an ISO11783 network.
    • Position, Rapid Update (PGN: 129025, 0x1F801)
    • COG & SOG, Rapid Update (PGN: 129026, 0x1F802)
    • Position Delta High Precision Rapid Update (PGN:129027, 0x1F803)
    • GNSS Position data (PGN: 129029, 0x1F805)
    • Rate of Turn (PGN: 127251, 0x1F113)
    • Datum (PGN: 129044, 0x1F814)
    • Vessel Heading (PGN 127250, 0x1F112)
  • Fixes multiple Fast Packet protocol issues
    • Fixed sending or receiving one extra packet when the total data count in a FP session was exactly on a frame boundary.
    • Fixed crash with some FP sessions where setting multipacket payloads would overrun the bounds of the message data vector.
    • Clarified a log message.
  • Added signed CANMessage data value getters
    • Added a way to get signed data values using the CANMessage class as an alternative to doing all the bit shifting and masking manually. This is similar to the already existing uint getters on the same class.

Caveats, etc

I don't really have a good way to test most of these messages to know if my scaling is correct (and coming across definitions for the scaling is... tricky...) so if anyone out there is willing, I'd love some help testing this with different GNSS receivers in the real world.

Please also let me know if you want additional messages added to this interface.

@ad3154 ad3154 added the enhancement New feature or request label Jul 14, 2023
@ad3154 ad3154 self-assigned this Jul 14, 2023
@ad3154 ad3154 temporarily deployed to Integrate Pull Request July 14, 2023 00:47 — with GitHub Actions Inactive
@ad3154 ad3154 force-pushed the nmea2000-messages branch from dc410ed to 2b5e60c Compare July 14, 2023 01:08
@ad3154 ad3154 temporarily deployed to Integrate Pull Request July 14, 2023 01:08 — with GitHub Actions Inactive
@ad3154 ad3154 force-pushed the nmea2000-messages branch from 2b5e60c to fe93775 Compare July 14, 2023 01:35
@ad3154 ad3154 temporarily deployed to Integrate Pull Request July 14, 2023 01:35 — with GitHub Actions Inactive
@ad3154 ad3154 marked this pull request as ready for review July 14, 2023 01:46
@ad3154 ad3154 requested a review from GwnDaan July 14, 2023 01:46
@GwnDaan
Copy link
Member

GwnDaan commented Jul 24, 2023

I can try to test this interface later this week, but I've been quite busy lately so I can't really promise anything. Sorry for the delay

@ad3154 ad3154 force-pushed the nmea2000-messages branch from fe93775 to e90eb49 Compare July 26, 2023 01:26
@ad3154 ad3154 temporarily deployed to Integrate Pull Request July 26, 2023 01:26 — with GitHub Actions Inactive
@GwnDaan GwnDaan force-pushed the nmea2000-messages branch from e90eb49 to 0494003 Compare August 3, 2023 10:09
@GwnDaan GwnDaan temporarily deployed to Integrate Pull Request August 3, 2023 10:09 — with GitHub Actions Inactive
@ad3154 ad3154 force-pushed the nmea2000-messages branch from 0494003 to 297e7f4 Compare August 27, 2023 15:57
@ad3154 ad3154 temporarily deployed to Integrate Pull Request August 27, 2023 15:57 — with GitHub Actions Inactive
@ad3154 ad3154 temporarily deployed to Integrate Pull Request September 7, 2023 23:57 — with GitHub Actions Inactive
@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 8, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 31 Code Smells

83.1% 83.1% Coverage
0.0% 0.0% Duplication

warning The version of Java (11.0.17) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

GwnDaan
GwnDaan reviewed Oct 27, 2023
View reviewed changes
Copy link
Member

@GwnDaan GwnDaan left a comment

Choose a reason for hiding this comment

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

First of all, sorry for taking so extremely long for my review 😞. But now it's here, I can say you did some neat stuff! A few small thing I noticed here and there, but overall happy with this addition :)

For testing this I started of by making a 'generator' example, and reading the output with another lovely open-source project canboat. Using candump vcan0 | candump2analyzer | analyzer I then verified everything is correctly serialized as per their output.

After all was correct, I created an example for the other way around, i.e. a 'parser' program. That way also the de-serialization could be verified. There was little I had to change to make it all work, so props for that. I'll create a PR for my changes 🔥

isobus/include/isobus/isobus/nmea2000_message_definitions.hpp Outdated Show resolved Hide resolved
isobus/include/isobus/isobus/nmea2000_message_interface.hpp Outdated Show resolved Hide resolved
isobus/include/isobus/isobus/nmea2000_message_interface.hpp Outdated Show resolved Hide resolved
isobus/include/isobus/isobus/nmea2000_message_definitions.hpp Outdated Show resolved Hide resolved
isobus/src/nmea2000_message_interface.cpp Outdated Show resolved Hide resolved
isobus/include/isobus/isobus/nmea2000_message_definitions.hpp Show resolved Hide resolved
isobus/include/isobus/isobus/nmea2000_message_definitions.hpp Outdated Show resolved Hide resolved
isobus/include/isobus/isobus/nmea2000_message_definitions.hpp Outdated Show resolved Hide resolved
isobus/src/nmea2000_message_definitions.cpp Show resolved Hide resolved
@ad3154 ad3154 force-pushed the nmea2000-messages branch from fa5e62d to 61ea638 Compare October 28, 2023 00:35
ad3154 and others added 9 commits October 28, 2023 13:58
@ad3154
[NMEA2K]: Started adding a general NMEA2000 messaging layer
dbb55ce 
Started adding message definitions and storage for some more common
NMEA2000 messages that might be found on an ISOBUS.
@ad3154
[NMEA2K]: Add message serializers, doxygen
9a06492 
Added serialize functions for each NMEA2K message data object.
Added more doxygen.
Added ways to get both raw and scaled values for most things.
@ad3154
[Core]: Add signed CANMessage data value getters
22f7bb6 
Added a way to get signed data values using the CANMessage class
as an alternative to doing all the bit shifting and masking manually.
This is similar to the already existing uint getters on the same class.
@ad3154
[NMEA2K]: Started adding NMEA2K messaging interface
3118329 
Added a class that will provide a nice common interface for common
NMEA 2000 messages that might be found on an ISOBUS.
@ad3154
[Core]: Add an additional bounds check for accessing the CF table on …
318f775 
…CF destruction

Fixed a rare crash that might happen if a CF address became 0xFE at
destruction time.
@ad3154
[NMEA2K]: Finished NMEA2000 message interface
4fc22aa
@ad3154
[NMEA2K]: Fixed some log spam and spelling errors
ff61284
@GwnDaan @ad3154
fix: incorrect exponents used for some nmea2000 message definitions
8b85bfb 
feat: extra raw/converted message field getters
@GwnDaan @ad3154
examples: add nmea2000 messsage parser/generator
f36422c
ad3154 added 2 commits October 28, 2023 13:58
@ad3154
[NMEA2K]: Tidied up spelling of CourseOverGroundReference
3973e97
@ad3154
[NMEA2K]: Altered value change detection to ignore timestamp
7cd8c88 
Fixed every NMEA2K message triggering a callback/event because
the timestamp was being considered as something that changed.
@ad3154 ad3154 force-pushed the nmea2000-messages branch from 23bb9cf to 7cd8c88 Compare October 28, 2023 20:11
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 40 Code Smells

80.9% 80.9% Coverage
0.0% 0.0% Duplication

@ad3154 ad3154 merged commit 96a58c3 into main Oct 28, 2023
7 checks passed
@ad3154 ad3154 deleted the nmea2000-messages branch October 28, 2023 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GwnDaan GwnDaan GwnDaan approved these changes

Assignees

@ad3154 ad3154

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ad3154 @GwnDaan