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

Feat: add satellites definition to GNSS object in navigation group (non-breaking) #572

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fabdrol
Copy link
Member

@fabdrol fabdrol commented May 10, 2020

This PR implements the following paths in the navigation group, with the goal of making it possible to express the satellites of a GNSS fix in Signal K. Use cases include:

  • Making more informed decisions on the quality of a fix
  • Showing a polar graph with all satellites relative to the receiver
  • Etc

The PR would add the following paths to the specification, without breaking anything else:

navigation.gnss.satellites.<PRN code>.elevation
navigation.gnss.satellites.<PRN code>.pseudoRandonNoiseCode
navigation.gnss.satellites.<PRN code>.azimuth
navigation.gnss.satellites.<PRN code>.signalToNoiseRatio

Satellites are identified by PRN code, which is unique to each satellite. signalToNoiseRatio has a unit dB (decibells), which is not a SI base unit, but sanctioned by SI anyway.

@fabdrol fabdrol requested a review from a team May 10, 2020 14:16
@fabdrol fabdrol self-assigned this May 10, 2020
@tkurki
Copy link
Member

tkurki commented May 10, 2020

Imho all spec additions need corresponding sample/test data, making sense of json schema is hard enough.

It says satellites in view. How do satellites get purged from full?

What about when you have more than one gps?

Is the data set the same regardless of the positioning system?

I would like to see this addition tied to concrete implementations in n2k-signalk and nmea0183 parser.

Using numberValue puts source data and timestamps under each individual value. Aren’t all the values transmitted as a single message?

Furthermore it would be great to exercise this data to create a sample ui, perhaps a map of the sky, like gps Devices show?

@fabdrol
Copy link
Member Author

fabdrol commented Jul 8, 2020

@tkurki we had some discussion about the merits of providing test data and use cases as you suggested in your post. I think it's a bad order to provide implementation before specification, as I strongly believe that implementation should follow specification. Also, I think that demanding implementation in order to test specification will slow down changes massively.

That said, I will commit to making PRs in n2k-signalk and nmea0138 parser once this is approved and part of the specification.

In order to answer your questions about GPS data (which are relevant to this PR, imo), I've asked @paul_dy to weigh in on the questions about GPS systems in general, and about test data.

@sumps
Copy link
Contributor

sumps commented Jul 8, 2020

Here is some test data - from our GPS160 in "Tri-Nav" professional mode which shows the latest GNSS sentences. Note the different Talker IDs - GP = GPS, GA = Galileo, GL = GLONASS and GN = GNSS (combined). The actual position sentences are GN as they are calculated using all three constellations, while the GSV sentences have the relevant talker ID based on which satellite signals it is related to.

I could set the GPS160 to a GPS, GALILEO or GLONASS only mode and then the Talker ID for all sentences would be GP, GA or GL (as applicable).

GPS160-Pro-Mode.zip

The GSA sentence lists all satellite PRNs for the satellites used in the position fix, while the GSV sentences show the status of all of the satellites currently in view. You would need to purge all no longer used satellites from Full, every second so Full only lists the satellites currently in view.

The datasets NMEA0183 or NMEA2000 should be the same for all GNSS systems, assuming correct adoption of the standard.

If you had multiple GNSS systems on the boat, each one could be outputting this level of data and so you would need to support multiple sources.

@sbender9
Copy link
Member

sbender9 commented Jul 8, 2020

I’m agreement that this is the time to go and do the implementation. That way you can validate that the spec is complete before merging it. Too many times something comes up in implementation that was missed when writing the spec.

@tkurki
Copy link
Member

tkurki commented Jul 12, 2020

additions need corresponding sample/test data, making sense of json schema is hard enough.

I was referring to example/test full & delta data in this comment. We have previously merged schema PRs that were not actually doing what they were supposed to. TDD, you know.

And I would start the TDD process from real world data in 0183 parser, now that Paul has graciously provided some.

If I were to start SK in Github today I'd seriously consider a monorepo approach. Then the NMEA0183, N2K and specification changes could be a single PR, bringing the RI and specification forward in atomic steps. Not that it needs atomicity, just trying to demonstrate the thinking that the spec and RI should move forward together. We still have inconsistencies that you've witnessed yourself like SignalK/signalk-server#1076 from adding stuff to the specification and failing to match it in implementation.

See #562 for an example of trying to practise what I'm preaching here.

slow down changes massively

Things are slowed down by not following up on the PRs and/or getting stuck in debate. My questions above still remain unanswered...

Copy link
Member

@tkurki tkurki left a comment

Choose a reason for hiding this comment

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

@tkurki
Copy link
Member

tkurki commented Jan 2, 2021

See #163 for earlier work in this direction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants