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.

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

Gatehouse wrapper messages feature #91

Closed
SiggyF opened this issue Nov 25, 2022 · 8 comments
Closed

Gatehouse wrapper messages feature #91

SiggyF opened this issue Nov 25, 2022 · 8 comments
Assignees
Labels
enhancement New feature or request

Comments

@SiggyF
Copy link

SiggyF commented Nov 25, 2022

Some AIS messages have so-called Gatehouse wrappers. These encapsulating messages contain extra information, such as time and checksums. Some readers also process these. See some more documentation here.

We have created a small mixin that allows for storing this extra information. We also added an extra message type to parse the metadata lines. Could you have a look to see if this is an approach you would be willing to adopt?

As an example, see the following, which is followed by a regular !AIVDM message

$PGHP,1,2020,12,31,23,59,58,239,0,0,0,1,2C*5B

I will submit the code as a PR referring to this issue.

@M0r13n
Copy link
Owner

M0r13n commented Nov 25, 2022

@SiggyF

Thank you!

I saw such messages already, but I never found the time to actually look at them in detail. So I am glad that you did. I will take a look at your PR this weekend. 😄

@M0r13n
Copy link
Owner

M0r13n commented Nov 27, 2022

@SiggyF

I took a look at your PR. Overall, I liked your suggested changes. But there are some details, that I would have done differently:

  • tests are missing or the existing tests even fail
  • you added a PGHPMessage class as a mixin to every AIS message
  • assertions are not a very good practice, because these are removed if optimizations are requested from Python

So, I played a bit around with your code and came up with a slightly different approach. I decided to make the parsing/decoding of NMEA messages a bit more generic. PHGP, AIVDM, and AIVDO messages are all NMEA messages - among many others. All these NMEA messages have in common, that they have a start delimiter, followed by a comma-separated sequence of fields, followed by the character '*', the checksum and an end-of-line marker. Therefore, I added a new class NMEASentence. This class encapsulates a single NMEA sentence and handles the basic stuff. This allows other classes to build upon this class.

Then there are two new classes:

  • GatehouseSentence: encapsulates PGHP messages
  • AISSentence: encapsulates AIS messages

I researched GH messages and came to the conclusion that:

GH internal messages type 1 are used for "encapsulating" NMEA messages.

The important part is that GH messages can be used to encapsulate every NMEA message - not just AIS messages. Thus, I decided to add a .meta attribute to all NMEASentences. This differs from your approach of adding the .meta attribute to every AIS message individually.

I am curious about your feedback. Can you live with my suggestions?

@Inrixia: Would you take a look at my changes? Your feedback has proven invaluable in the past. 😁

@Inrixia
Copy link
Contributor

Inrixia commented Nov 27, 2022

Will take a look when I get the time 👍

@Inrixia
Copy link
Contributor

Inrixia commented Dec 3, 2022

Changes in #93 look good.

Might be worth considering a better name than meta and it's associated functions for easier readability.

@M0r13n
Copy link
Owner

M0r13n commented Dec 3, 2022

@Inrixia

Thanks for looking at my changes.

Might be worth considering a better name than meta

Do you have any good suggestion? I thought about this also and did not came up with a nice naming scheme. I ultimately decided to use meta, because:

  • it's kind of meta data about the message
  • it's somewhat generic: this allows to store other kinds of "meta data"

@M0r13n M0r13n self-assigned this Dec 11, 2022
@M0r13n M0r13n added the enhancement New feature or request label Dec 11, 2022
@M0r13n
Copy link
Owner

M0r13n commented Dec 11, 2022

@Inrixia

I renamed the newly introduced .meta attribute to wrapper_msg. I think this makes the intention clear.

@SiggyF I closed your original PR. But I used much of your prior work to implement support for gatehouse wrappers. Thank you!

With Version 2.3.0 gatehouse messages are parsed by every stream class. See examples

@M0r13n M0r13n closed this as completed Dec 11, 2022
@Inrixia
Copy link
Contributor

Inrixia commented Dec 12, 2022

@M0r13n Perfect, that is much more understandable

@SiggyF
Copy link
Author

SiggyF commented Jan 5, 2023

Thanks for implementing this feature and for the suggestions. We will test it with our datastreams.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants