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

APB and BOD sentences not decoded according to SignalK spec #239

Open
tvr256 opened this issue Mar 1, 2023 · 8 comments
Open

APB and BOD sentences not decoded according to SignalK spec #239

tvr256 opened this issue Mar 1, 2023 · 8 comments

Comments

@tvr256
Copy link

tvr256 commented Mar 1, 2023

I've noticed a number of areas where the APB and BOD parsers don't follow the SignalK spec:

The APB parser incorrectly stores "bearing to next waypoint" to navigation.courseRhumbline.bearingToDestinationTrue which is not defined in the SignalK spec. Instead, it should be stored to navigation.courseRhumbline.nextPoint.bearingTrue.
(Note, the RMB sentence also contains the same value and already stores it to the correct SignalK field).

Both the APB and BOD parsers incorrectly store "bearing from previous waypoint to next waypoint" to navigation.courseRhumbline.bearingOriginToDestinationTrue which is not defined in the SignalK spec. Instead, it should be stored to navigation.courseRhumbline.bearingTrackTrue.

The APB sentence stores the next waypoint ID to navigation.courseRhumbline.nextPoint.ID. This field isn't mentioned in the SignalK spec, should it be added?

The confusion is probably due to the NMEA spec mentioning "origin waypoint" and "destination waypoint". They actually mean previous and next waypoint, but it could easily be misinterpreted as the route starting point and final destination.

@tvr256 tvr256 changed the title APB not decoded according to SignalK spec APB and BOD sentences not decoded according to SignalK spec Mar 1, 2023
@Techstyleuk
Copy link
Contributor

With regard to "navigation.courseRhumbline.nextPoint.ID" what should it be? if we get a consensus, I will fix APB

@tvr256
Copy link
Author

tvr256 commented Feb 16, 2024

Thanks for fixing! With regards to "navigation.courseRhumbline.nextPoint.ID", I think the code is doing the right thing but it needs to be added to the SignalK courseRhumbline spec

Techstyleuk added a commit to Techstyleuk/specification that referenced this issue Feb 18, 2024
Updating the specification to be inline with how its parsed in some of the nmea0183-signalk hooks.  for example, APB.  see: SignalK/nmea0183-signalk#239
This is a partial fix
@Techstyleuk
Copy link
Contributor

Techstyleuk commented Feb 18, 2024

The Code for APB has both "bearingTrackTrue" and "bearingOriginToDestinationTrue" this could be for legacy reasons, so I am reluctant to remove it unless @tkurki says it is OK to remove? See the Current code below:

` {

        path: `navigation.courseRhumbline.bearingTrack${bearingOriginToDestType}`,

        value: bearingOriginToDest,

      },

      {
        path: `navigation.courseRhumbline.bearingOriginToDestination${bearingOriginToDestType}`,

        value: bearingOriginToDest,

      },`

BOD already has:
` {

        path: 'navigation.courseRhumbline.bearingTrackTrue',

        value: bearingOriginToDestination.True || null,

      },

      {

        path: 'navigation.courseRhumbline.bearingTrackMagnetic',

        value: bearingOriginToDestination.Magnetic || null,

      },`

Is this part fixed already?

@tvr256
Copy link
Author

tvr256 commented Feb 19, 2024

I've just tested BOD and it looks correct to me (except the waypoint ID isn't in the SignalK spec). So either its been fixed or I raised the bug in error

$INBOD,123.45,T,124.56,M,NEXT_WP_ID,PREVIOUS_WP_ID*44

image

@tvr256
Copy link
Author

tvr256 commented Feb 19, 2024

I've just retested APB, here's what I've found:

$INAPB,A,A,123.45,R,N,A,A,234.56,T,DEST_WP_ID,345.67,M,346.78,T,A*6E

Incorrectly mapped to SignalK:

  • APB Bearing to Destination Magnetic -> SignalK navigation.courseRhumbline.bearingToDestinationMagnetic
    -- SignalK spec states navigation.courseRhumbline.nextPoint.bearingMagnetic

  • APB Bearing to Destination True -> SignalK navigation.courseRhumbline.bearingToDestinationTrue
    -- SignalK spec states navigation.courseRhumbline.nextPoint.bearingTrue

  • APB Heading to Steer Magnetic -> SignalK steering.autopilot.target.headingMagnetic
    -- SignalK spec states steering.autopilot.targetHeadingMagnetic

  • APB Heading to Steer True -> SignalK steering.autopilot.target.headingTrue
    -- SignalK spec states steering.autopilot.targetHeadingNorth

  • APB Bearing Origin to Destination True -> SignalK navigation.courseRhumbline.bearingTrackTrue
    -- Also mapped to navigation.courseRhumbline.bearingOriginToDestinationTrue

  • APB Bearing Origin to Destination Magnetic -> SignalK navigation.courseRhumbline.bearingTrackMagnetic
    -- Also mapped to navigation.courseRhumbline.bearingOriginToDestinationMagnetic

Not mapped to SignalK

  • APB Positioning System Mode Indicator
    This also affects the GLL sentence parser, so I'll raise a separate defect for this

Correctly mapped to SignalK:

  • APB Status 1 : A = Data valid, V = Loran-C Blink or SNR warning. SignalK ignores the entire APB message if status is V
  • APB Status 2: A = Data valid or not used, V = Loran-C Cycle Lock warning flag. SignalK ignores the entire APB message if status is V
  • APB XTE magnitude -> SignalK navigation.courseRhumbline.crossTrackError
  • APB Direction to Steer : SignalK navigation.courseRhumbline.crossTrackError is negative if Direction to Steer is R
  • APB XTE Units : N (nautical miles) or K (km) are converted to m in SignalK navigation.courseRhumbline.crossTrackError
  • APB Arrival Circle Entered -> SignalK notifications.arrivalCircleEntered
  • APB Perpendicular Passed -> SignalK notifications.perpendicularPassed
  • APB Destination Waypoint ID-> SignalK navigation.courseRhumbline.nextPoint.ID

image

@Techstyleuk
Copy link
Contributor

OK, with regard to BOD, I will leave as is.

On APB, I can correct the .nextpoint.bearingTrue/Magnetic entries and the .autopilot.targetHeadingTrue/Magnetic but I will wait until @tkurki comments about removal of .courseRhumbline.bearingOriginToDestinationTrue/Magnetic as this may need to be left for legacy reasons

@tvr256
Copy link
Author

tvr256 commented Feb 19, 2024

Looks like the legacy field names were directly copied using the APB field name instead of the SignalK field name. So it could be a simple typo.

I agree we need to be careful removing fields in case someone is using the legacy field names. PyPilot springs to mind, I'll alert @seandepagnier

@panaaj
Copy link
Member

panaaj commented Feb 20, 2024

It would also be prudent to consider the Course API which maintains both the V1 paths and the V2 paths.

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

No branches or pull requests

4 participants