fix: communication.callsignVhf not set properly in baseDeltas.json

[sbender9]

Was: Fixes SignalK/n2k-signalk#277

See #1842.

[rhbvkleef]

This PR is breaking. I already depend on the old (incorrect) behaviour. I know know about it, but maybe placing a warning in the release notes about this breaking change might be useful for others.

[KEGustafsson]

Noticed the same issues. Old value object structure was changed to nested value, which breaks plugins using old format.

@sbender9 @tkurki Would this PR be OK to fix the issue?
KEGustafsson@c25d95d

[sbender9]

You're essentially proposing to revert this PR?

[sbender9]

I think we need to get plugin authors to change, since this is not a value object in the spec.

[KEGustafsson]

Ok, then it is clear violation against to the specs.

This is kind of breaking change. Current format, as it is now, have been there quite long time. Is it easier to change specs than plugins, databases, apps, etc...

[panaaj]

Also an observation, these deltas have a an empty path rather than "communication"

…

________________________________ From: Karl-Erik Gustafsson ***@***.***> Sent: Friday, December 6, 2024 6:06:15 AM To: SignalK/signalk-server ***@***.***> Cc: Subscribed ***@***.***> Subject: Re: [SignalK/signalk-server] fix: communication.callsignVhf not set properly in baseDeltas.json (PR #1832) Ok, then it is clear violation against to the specs. This is kind of breaking change. Current format, as it is now, have been there quite long time. Is it easier to change specs than plugins, databases, apps, etc... — Reply to this email directly, view it on GitHub<#1832 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AJF4C5LV3DIOVEAMMPYZ7PD2ECTK7AVCNFSM6AAAAABSHIBLJSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMRRGI2DANZSGU>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[sbender9]

Also an observation, these deltas have a an empty path rather than "communication"
…
________________________________ From: Karl-Erik Gustafsson @.> Sent: Friday, December 6, 2024 6:06:15 AM To: SignalK/signalk-server @.> Cc: Subscribed @.> Subject: Re: [SignalK/signalk-server] fix: communication.callsignVhf not set properly in baseDeltas.json (PR #1832) Ok, then it is clear violation against to the specs. This is kind of breaking change. Current format, as it is now, have been there quite long time. Is it easier to change specs than plugins, databases, apps, etc... — Reply to this email directly, view it on GitHub<#1832 (comment)>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AJF4C5LV3DIOVEAMMPYZ7PD2ECTK7AVCNFSM6AAAAABSHIBLJSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMRRGI2DANZSGU. You are receiving this because you are subscribed to this thread.Message ID: @.>

What do you mean?

[sbender9]

@tkurki what do you think?

This is a tough one. In general, I would not want to make breaking changes to the spec either. I would also much rather this was not a value object.

[KEGustafsson]

Also an observation, these deltas have a an empty path rather than "communication"

I think this has been reference for callSign rather than specs. No path, but only value object. Good notice!
https://github.com/SignalK/nmea0183-signalk/blob/master/hooks/VDM.js#L227-L232

[sbender9]

And of course, n2k-signalk does it per the spec. https://github.com/SignalK/n2k-signalk/blob/3b0401af0738c3fd8097ac661e060cdf4b9296cc/pgns/129794.js#L53

[sbender9]

Given that any plugins, etc. that use it are already broken for n2k sources, I think we should stick with the current spec and fix 0183.

[KEGustafsson]

Thats clear then. Nmea0183 and the rest using value object need to be updated.
What is target for next SK release? I think common target should be simultaneous updates for server, plugins & apps. Otherwise it will be a mess.

Just to be clear, this is now correct format?

{
  path: 'communication.callsignVhf',
  value: data.callsign,
}

[sbender9]

Yes, that's the correct format.

Unless @tkurki has a date in mind, we don't have any target.

We can definitely coordinate the releases.

[tkurki]

Oh, I missed this discussion and started digging into this from scratch in #1842 rather than continue in a merged PR.