Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v0 coexistence and data type extensibility #64

Merged
merged 36 commits into from
Dec 24, 2019
Merged

v0 coexistence and data type extensibility #64

merged 36 commits into from
Dec 24, 2019

Conversation

pavel-kirienko
Copy link
Member

@pavel-kirienko pavel-kirienko commented Nov 15, 2019

Fixes #59
Fixes #60
Fixes #63

Provides the necessary foundation for #61 to be added in the future (perhaps v1.1).

Serialization rules are modified as follows:

  • Unused data left over after a serialized representation is deserialized shall be ignored.
  • Missing data is substituted with zero bits.

The implicit truncation rule will allow us to introduce explicit support for subtyping in DSDL later without affecting backward compatibility. The implicit zero extension will enable us to introduce optional fields if that is found to be necessary, although as I described in #61 I hope to find a better solution than naive extensions. Together, the two rules shift UAVCAN on the strictness-extensibility spectrum far towards the latter end, thus resolving the extensibility and future-proofness issues discussed at https://forum.uavcan.org/t/uavcan-v1-0-and-ardupilot/671. Although this change comes at a cost for deeply isolated deployments which may not benefit from it, the cost is considered minor and the trade-off is in general beneficial for the ecosystem at large.

Data types are now explicitly allowed to be moved between namespaces and renamed. The validity of this used to be uncertain.

The transport layer implementation provides an explicit description of how to handle incoming transfers that are longer than the expected maximum. This is needed to support derived types and, shall we fail to find a better solution, extension fields. The transport spec is also explicit about short transfers not being an error.

The CAN ID layout has been modified to retain the old position of the node-ID field. This is suboptimal for the prospective migration to v2 in the far future, but it does solve the v0 coexistence problem and also provides the possibility to extend the service-ID field from the current 9 bits to 10 bits in the future if that is found to be necessary.

A pre-built PDF is attached: UAVCAN_Specification.pdf

Updated the CAN ID layout to accommodate the v0/v1 compatibility objectives outlined in this thread:
https://forum.uavcan.org/t/uavcan-v1-0-and-ardupilot/671.

The updated format keeps the source node-ID at the same offset regardless of the protocol version.
This allows v0/v1 nodes to share the same bus with no risk of CAN ID collision.

The change required me to move the version bit from the 0th position to the 23rd position.
The new position is chosen to allow easy expansion of the service-ID field shall that become necessary in the future.
Also, for message transfers, this bit was already effectively reserved, even though it has not been explicitly marked as such,
because the range of the 16-bit subject-ID field was limited to [0, 32767], meaning that the most significant bit,
which is 23rd, was to be always zero.

The message transfer format got an extra reserved bit at the 7th position to keep the subject-ID aligned.
@pavel-kirienko pavel-kirienko added enhancement New feature or request help wanted Extra attention is needed labels Nov 15, 2019
@pavel-kirienko pavel-kirienko added this to the v1.0 milestone Nov 15, 2019
@pavel-kirienko pavel-kirienko self-assigned this Nov 15, 2019
@pavel-kirienko pavel-kirienko changed the title Future v0 coexistence and data type extensibility Nov 15, 2019
pavel-kirienko added a commit to OpenCyphal/public_regulated_data_types that referenced this pull request Nov 16, 2019
…l subtypes of their counterparts under uavcan.si.unit.

This change is related to OpenCyphal/specification#64.

The breakage is acceptable because v1 is not yet used in production and is not yet released.
specification/dsdl/compatibility.tex Show resolved Hide resolved
specification/dsdl/compatibility.tex Show resolved Hide resolved
specification/dsdl/compatibility.tex Outdated Show resolved Hide resolved
specification/dsdl/serializable_types.tex Outdated Show resolved Hide resolved
specification/dsdl/serializable_types.tex Outdated Show resolved Hide resolved
specification/transport_layer/transport_layer.tex Outdated Show resolved Hide resolved
specification/transport_layer/transport_layer.tex Outdated Show resolved Hide resolved
specification/transport_layer/transport_layer.tex Outdated Show resolved Hide resolved
specification/transport_layer/transport_layer.tex Outdated Show resolved Hide resolved
specification/transport_layer/transport_layer.tex Outdated Show resolved Hide resolved
Copy link
Member

@thirtytwobits thirtytwobits left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm okay with this change but I need a bit more information about how this actually enables v0/v1 interoperability (i.e. how do you know that a given frame is a v0 versus v1 frame) and that we aren't limiting portability to ethernet or other network layers. Specifically, are we taking a hard dependency on a network having a statically defined MTU because of the payload truncation rule?

pavel-kirienko and others added 4 commits December 11, 2019 10:48
Co-Authored-By: Scott Dixon <dixonsco@amazon.com>
Co-Authored-By: Scott Dixon <dixonsco@amazon.com>
Co-Authored-By: Scott Dixon <dixonsco@amazon.com>
Co-Authored-By: Scott Dixon <dixonsco@amazon.com>
@pavel-kirienko
Copy link
Member Author

how do you know that a given frame is a v0 versus v1 frame

See the link I posted in the comments here. The short answer is that we look at the toggle bit.

are we taking a hard dependency on a network having a statically defined MTU because of the payload truncation rule?

No. Could you please clarify which section may lead one to think that there is a dependency on the MTU?


There is a subtle problem arising from the implicit zero extension rule: if a transport layer requires implicit padding, it shall be done with zero bits only. Otherwise, an implementation that attempts to deserialize an object using its subtype may consume the non-zero padding bits mistaking them for useful payload. This is somewhat suboptimal because of the stuff bits on CAN FD, but it is necessary for extensibility. I am going to update the PR with that change; we can revise it if necessary.

…lows from the discussion at #64 (comment). The wording may be amended slightly in the upcoming review of the transport layer chapter but the changes are expected to be minor.
@pavel-kirienko
Copy link
Member Author

After this one is merged, the next step is to restructure the transport layer chapter (I started this on a separate branch already but I will need to start over because of the significant changes introduced here) and to simplify the data type compatibility rules.

Copy link
Member

@thirtytwobits thirtytwobits left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of non-critical comments.

specification/dsdl/compatibility.tex Outdated Show resolved Hide resolved
specification/dsdl/compatibility.tex Outdated Show resolved Hide resolved
specification/dsdl/compatibility.tex Outdated Show resolved Hide resolved
pavel-kirienko and others added 2 commits December 24, 2019 09:21
Co-Authored-By: Scott Dixon <dixonsco@amazon.com>
Co-Authored-By: Scott Dixon <dixonsco@amazon.com>
@pavel-kirienko
Copy link
Member Author

I am going to merge this now to unblock my further progress. If you want to follow-up on #64 (comment), let's register it as a separate issue.

@pavel-kirienko pavel-kirienko merged commit 8408188 into master Dec 24, 2019
@pavel-kirienko pavel-kirienko deleted the future branch December 24, 2019 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
2 participants