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

SeaTalk composite data: implementation with session #156

Merged
merged 8 commits into from
Aug 20, 2019

Conversation

raffmont
Copy link
Contributor

The use of 0x50, 0x51 (latitude, longitude) and 0x54, 0x56 (time, date) datagrams have been implemented using the session propagated by the Parser class.

@tkurki
Copy link
Member

tkurki commented Aug 19, 2019

STALK tests are now failing, you probably got a message about them? Please fix: make sure npm test succeeds on your machine.

@raffmont
Copy link
Contributor Author

The tests failed because not compliant with the new 0x50 0x51 behaviour and because 0x54 and 0x56 have been disabled in order to prevent a misbehaviour.
I updated the tests in order to match the new implementation.

Copy link
Contributor

@joabakk joabakk left a comment

Choose a reason for hiding this comment

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

I have no means of testing these changes, but they look ok to me. Could you rebase with sensible commit titles (such as fix: STALK lat/lon composite values and fix: STALK time/dat composite values and any changes to the tests as three separate commits)?

@tkurki
Copy link
Member

tkurki commented Aug 20, 2019

No need to fix the commits, easy enough to use Github SquashAndMerge, since this can be just a single commit.

@tkurki tkurki merged commit ec2e6d9 into SignalK:master Aug 20, 2019
@joabakk joabakk mentioned this pull request Jan 27, 2021
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.

3 participants