Sub-IoT-Stack icon indicating copy to clipboard operation
Sub-IoT-Stack copied to clipboard

ALP_ITF_ID_D7ASP interface config format doesn't seem to match spec (same with pyd7a impl)

Open rdaum opened this issue 2 years ago • 2 comments

D7A Specification Version 1.2 page 54, section 9.2.1 describes the Configuration of consisting of 4+ bytes with the first three bytes being QoS, TO, and TE, then followed by addressee.

What's actually emitted at https://github.com/Sub-IoT/Sub-IoT-Stack/blob/master/stack/modules/alp/alp.c#L151

Puts out 1 byte less, with just qos, dormant_timeout, and then addressee. No 'TE' byte.

Likewise the python implementation @ https://github.com/Sub-IoT/pyd7a/blob/master/d7a/sp/configuration.py#L46

Also puts out 1 byte less than the spec and is missing the 'TE' byte.

Writing an implementation that follows the specification literally, as the author of https://github.com/Stratus51/rust_dash7_alp has done, seems to lead to problems.

rdaum avatar May 26 '22 20:05 rdaum

Hi Ryan,

it seems you are right! We tried to follow the implementation as close as possible but seemed to have missed this field.

I will take a look at it and try to make a backwards compatible version for this not to break implementations relying on the current version.

In the newest, not yet released, version of the spec, we also opted to add a version field to this configuration to make sure we can solve issues like these easier.

Thank you for investigating!

Liam

LOorts-Aloxy avatar May 30 '22 06:05 LOorts-Aloxy

Ryan,

It seems this is a remnant from a previous version of the spec which did not yet include this field. Perhaps it's best to add this field under a cmake variable to ensure people can keep using the previous structure to not break backwards compatibility. In the newest (unreleased) version, byte 2 will be a version field if a flag is set in the first byte: image This is not yet released but could be taken into account for this fix as it will be included in the next spec.

Liam

LOorts-Aloxy avatar May 30 '22 13:05 LOorts-Aloxy