latest master (5f926c) broke emergency_routes_spec.rb test

[sveitech]

In the latest master branch of the validator, the following update has been inserted into command_helpers.rb:

disable_emergency_route route
    wait_for_status(@task,
      "emergency route #{route} to be disabled",
      [
        {'sCI'=>'S0006','n'=>'status','s'=>'False'},
        {'sCI'=>'S0006','n'=>'emergencystage','s'=>route}
      ]
    )

This brought to light a possible issue with the specification for command M0005 and status S0006. In command M0005 (https://rsmp-nordic.org/rsmp_specifications/rsmp_sxl_traffic_lights/1.0.15/sxl_traffic_light_controller.html#m0005) one emergency route can be enabled or disabled, and S0006 (https://rsmp-nordic.org/rsmp_specifications/rsmp_sxl_traffic_lights/1.0.15/sxl_traffic_light_controller.html#s0006) reflects this.

So, according to the test-update, S0006 should always reflect the last known "order" to M0005? I.E. if M0005 was ordered to disable emergency route 5, S0006 sould read false/5.

Can multiple emergency routes be set? Is it a valid command to set emergency 1,2,3 to true for example? If so, what should S0006 contain?

There also seems to be an issue with the wording: S0006 is "emergency stage" and M0005 is "emergency route". The validator test expects "emergencystage" to be equal to route. So, are "stage" and "route" the same?

[emiltin]

Hi, the issue title indicates that something broke. But reading the issue, I'm not sure I understand what it is. {'sCI'=>'S0006','n'=>'emergencystage','s'=>route} was added because we need to check that the route is actually activated.

[sveitech]

The function shown is called "disable_emergency_route". It disables the route, and waits for the status of the route to be "False". But it also expects a certain route number within the function.

So this got us wondering, what the actual functionality of S0006 should be, if it should always reflect the last order sent to M0005 ?

[emiltin]

I hope @otterdahl can provide input. But my understanding is that only one emergency route can be active at the same time, and that route=stage (and I think we should change 'stage' to 'route' in the spec).

[otterdahl]

So, according to the test-update, S0006 should always reflect the last known "order" to M0005? I.E. if M0005 was ordered to disable emergency route 5, S0006 sould read false/5.

Can multiple emergency routes be set? Is it a valid command to set emergency 1,2,3 to true for example? If so, what should S0006 contain?

Well, we've assumed that only one emergency route can be set at a time. And only a single emergency route can given in S0006. But now that you mention it, it would be useful to include all the routes in S0006 if your controller can have several active at a time. Perhaps we can extend S0006 in an upcoming version to also include an array of routes, but keep the existing for backwards compatibility.

As of SXL 1.1: Which active route to set in S0006 (if there is more than one active) is not defined, but the validator test will not test activating multiple routes at the same time (at least not for SXL 1.1). So as far as the validation goes, only one route is activated and only one is expected to be returned in S0006.

There also seems to be an issue with the wording: S0006 is "emergency stage" and M0005 is "emergency route". The validator test expects "emergencystage" to be equal to route. So, are "stage" and "route" the same?

Yes, this is unfortunate. We have used "stage" and "route" in the same context. We should fix this, but we can't change the format without breaking backwards compatibility, but we can add a clarification in the description text.

[emiltin]

I think this is the behaviour that the test currently expects? Only one route can be active, and that's the one returned by S0006.

[otterdahl]

I think this is the behaviour that the test currently expects? Only one route can be active, and that's the one returned by S0006.

Yes

[emiltin]

I tihnk this issue can be closed, since the tests reflect the current expected behaviour?

[emiltin]

@otterdahl do we want to amend the 1.1 sxl to allow empty strings for emergencystage?

[otterdahl]

@otterdahl do we want to amend the 1.1 sxl to allow empty strings for emergencystage?

Given that S0006 doesn't clearly specify what to send if there is no emergency stage configured, I think this is an acceptable solution.

[emiltin]

Perhaps emergencystate=null is better? I think it's easier to define in our sxl.yaml and validate, and also generally clearer.

[emiltin]

Perhaps emergencystate=null is better? I think it's easier to define in our sxl.yaml and validate, and also generally clearer.

The Core spec currently requires s to be a string, so using nulls would require a change to the core spec.

That leaves using either "" or 0.

[emiltin]

Schema, gem and validator now agree that for S0006, emergencystate should be zero if status=0. Validator PR in #360.
Note that S0006 is depreciated from 1.2, replaced by S0035.
I believe this can be closed.