rsmp-nordic / rsmp_schema

JSON Schema for automatically validating RSMP messages
MIT License
1 stars 0 forks source link

Add S0035 "emergency route" for supporting multiple active routes #68

Closed otterdahl closed 1 year ago

otterdahl commented 1 year ago

Discussion in https://github.com/rsmp-nordic/rsmp_validator/issues/270

Needs further discussion before merging

emiltin commented 1 year ago

I suggest we explain more clearly how multiple emergency routes work; I think this includes more explanation in M0005. I think S0006 should be depreciated if we add this new S0035.

A few issues regarding this new S0035

otterdahl commented 1 year ago
  • Does it make sense to have multiple emergecy routes, from a traffic point of view?

Yes, I think it makes sense. I knew several could be configured, but I didn't think multiple could be active as once. A guess it could be true.

  • But does an array or a hash imply that you need to send all route everytime one of them changes? Or should only active routes be send? Or only routes that changed state since last update?

Only send active routes seems like a good idea.

  • How does update rate work with multiple routes?

If we send all active routes with each update the supervision system doesn't need to keep track of which changes with each update in order to display a complete list of active routes.

emiltin commented 1 year ago

Does this new S0035 imply that the existing S0006 does NOT support multiple routes? That would simplify our attempts to amend the existing 1.1 and older schemas.

emiltin commented 1 year ago

Only send active routes seems like a good idea.

If we send only active routes, then all we need is a list of integers:

[1, 3, 7]

This is a type of delta - you send only what changed since last update. This minimizes data size, but requires the receiver to keep the full state and update it based on the delta values received.

The alternative is to send a full data set. This requires more data, but the receiver does does not need to handle deltas.

I feel this balance between delta vs. full status is something that should be handled in a general way for all status messages, like update rate is handled in the same way. Perhaps something to consider for v4 of the core spec.

otterdahl commented 1 year ago

I've updated the draft to only contain active routes

The alternative is to send a full data set. This requires more data, but the receiver does does not need to handle deltas.

I feel this balance between delta vs. full status is something that should be handled in a general way for all status messages, like update rate is handled in the same way. Perhaps something to consider for v4 of the core spec.

My personal opinion is that deltas should be avoided, unless there is a strong argument supporting it. We've seen cases where the supervision system looses track of the current state from time to time for whatever reason (network issues, system restarts, whatever..). Any subscriptions should be maintained during network outages (if it is configured that way), but the buffer size is limited and figuring out the current state from buffered data is not without potential issues. I suppose we could specify that the initial value after a new subscription should contain the complete data, and the supervision system can unsubscribe/subscribe again, but it would add complexity to the protocol.

otterdahl commented 1 year ago

Does this new S0035 imply that the existing S0006 does NOT support multiple routes?

Yes. I think so. S0006 is badly suited for multiple routes. It not clear how to use it unless we use comma separated lists or something, which we better avoid.

emiltin commented 1 year ago

OK, I think we agree that S0006 can only handle one active route.

SwarcoPalm commented 1 year ago

I've updated the draft to only contain active routes

The alternative is to send a full data set. This requires more data, but the receiver does does not need to handle deltas. I feel this balance between delta vs. full status is something that should be handled in a general way for all status messages, like update rate is handled in the same way. Perhaps something to consider for v4 of the core spec.

My personal opinion is that deltas should be avoided, unless there is a strong argument supporting it. We've seen cases where the supervision system looses track of the current state from time to time for whatever reason (network issues, system restarts, whatever..). Any subscriptions should be maintained during network outages (if it is configured that way), but the buffer size is limited and figuring out the current state from buffered data is not without potential issues. I suppose we could specify that the initial value after a new subscription should contain the complete data, and the supervision system can unsubscribe/subscribe again, but it would add complexity to the protocol.

I agree wholeheartedly that deltas should be avoided. Considering the massive overhead of transferring human readable JSON in RSMP, it seems strange to try to save a few bytes by giving deltas. There’s never going to be hundreds of emergency routes configured, maximum a handful.

otterdahl commented 1 year ago

This PR (which is also staged to be included with the next version) adds the new argument (statusByIntersection) to the existing S0005 status.

In order to stay more consistent we can consider add this new status the existing S0006 and avoid creating a completely new status.

otterdahl commented 1 year ago

I'm merging this so we can review the online draft. Closing this