rsmp-nordic / rsmp_core

RSMP core specification
MIT License
6 stars 1 forks source link

Content of "rea" with standardized "core" error codes #132

Open marcgarba opened 1 year ago

marcgarba commented 1 year ago

It would be interesting to have an error code part, before the error string. And with some standardized error code values. I've also seen this issue sugesting adding a new tag "reaCode": #91 But if not wanted to avoid breaking "core" specifications, we could simply add an error code at the start of the current "rea" string like this:

"x: yyyyyy..." with

A supervisor (or another equipment) could just make use of the value before ":" separator character. A list of standardized errors codes should be defined at the RSMP "Core" level in the specifications.

Also each SXL could define some errors adapted to its field. Perhaps adding an "error" sheet in the Excel document could be interesting (for information, not linked with yaml). To avoid numbering clash with RSMP "Core" list, adding letters prefix before the error code could be used. The TLC SXL should not have any RSMP "core" errors part included (only keeping plans errors, I/O errors, ...).

Ideas of errors for a standardized list for RSMP "Core" we could retrieve in the specifications :

"1: failed to desereliaze packet"
"2: error on bad packet mType not 'rSMsg'"
"3: unknown message type"
"4: missing/error for a JSON element"
"5: RSMP version requested not supported"
"10: unknown site"
"11: site already connected"
"20: SXL version requested not supported"
"30: for an alarm, failed to find aCId"
"31: for an alarm, unknown alarm specialisation"
"40: for a status, unknown sCI"
"41: for a status, unknown argument name"
"42: for a status, invalid or out of range value"
"50: for a command, unknown cCI"
"51: for a command, unknown argument name"
"52: for a command, incomplete arguments"
"53: for a command, invalid or out of range value"

The string error description can be dynamically generated with more or less details.

Example for an incompatible RSMP version: "5: RSMP versions [3.2] requested, but only [3.1.1,3.1.2,3.1.3,3.1.4,3.1.5] supported"

Example for an unknown message type: "3: unknown message type 'StatusReq'"

emiltin commented 11 months ago

I would much prefer adding a reaCode, so that it can be validated with JSON Schema.

emiltin commented 11 months ago

It would be possible to to have just a code, but sometimes it might be restrictive. I think a code + decrription is best. Codes would be strings (enums), like 'unknownMessage', 'connectionError', etc.

marcgarba commented 11 months ago

The problem of adding a new reaCode is breaking current RSMP Core. So for now, we have to do with the single rea string available.

About the code value, isn't a simple numeric value more flexible for evolutions ? (not needing to enum all the possible strings errors exclusively).

emiltin commented 11 months ago

I think we do want to enumerate all the codes, because that means we can validate the schema. That also means adding new codes if needed.

Whether we use an integer or an enum string, doesn't make a big difference in how we maintain the spec. But an integer has a smaller footprint, while a string might be easier to read.

marcgarba commented 11 months ago

Thank you for these enlightening remarks.

So if keeping for now with the only rea, the example for an unknown message type could become : "unknownMessage: unknown message type 'StatusReq'"

emiltin commented 11 months ago

Right. It's a bit of a hack to encode stuff into the reason string, and I would generally discourage it, but if you don't want to wait for the core spec to support a reasonCode, it's a possibility.

marcgarba commented 1 month ago

After some discussions on our side, people think it would be better to have an integer value for a new "reaCode".

This results in a very compact value, well suited to computer use (when some values (or range of values) have to been tested), and for humans the existing "code" fileds allows to have a direct and complete easy-to-understand description.

Numeric values should be classified according to different ranges of values corresponding to meanings, and with a numeric value, it will be possible to add new values without having to modify a long list of enums strings. With ranges values principle, even if not knowing exact cause, we can know the global meaning.

We have an example with the http errors code list : https://en.wikipedia.org/wiki/List_of_HTTP_status_codes

emiltin commented 1 month ago

I seems we agree it's best to add new reaCode enum attribute. Therefore I'm closing this in favour of #91