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

Add support for strings and byte arrays #97

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

pavel-kirienko
Copy link
Member

@pavel-kirienko pavel-kirienko commented May 2, 2023

Implement string and byte array support as described in OpenCyphal/specification#51. The Specification will be updated after this is agreed upon. The grammar has been modified as follows:

  • Add utf8 primitive.
  • Add byte primitive.
  • Remove the unnecessary form saturated bool, which was shorthand for bool, for regularity. From now on, only unqualified bool is allowed. The expected impact is zero since the long form was never seen in the wild. Note that truncated bool was never legal.

At the moment, the following constraints are enforced; they may be lifted in the future:

  • utf8 can only be used as an element type of a variable-length array:
utf8[<=10] name  # valid
utf8[10]   name  # invalid
utf8       name  # invalid
  • byte can only be used as an element type of an array:
byte[<=10] data  # valid
byte[10]   data  # valid
byte       data  # invalid

Both utf8 and byte are concrete instances of UnsignedIntegerType. By virtue of this, existing users of PyDSDL (like Nunavut) will interpret both utf8 and byte as truncated uint8, until explicitly updated to take advantage of the new feature. That is:

utf8[<=10] name  # seen by Nunavut as: truncated uint8[<=10]
utf8[10]   name  # frontend error
utf8       name  # frontend error
byte[<=10] data  # seen by Nunavut as: truncated uint8[<=10]
byte[10]   data  # seen by Nunavut as: truncated uint8[10]
byte       data  # frontend error

Property pydsdl.ArrayType.string_like is now deprecated and should be replaced with an explicit isinstance(array.element_type, pydsdl.UTF8Type).

Some internal refactoring has been done to unify the deprecation consistency checking with the aggregation constraints listed above (e.g., utf8[<=10] is valid but utf8[10] is not).

Closes #96
Relates to OpenCyphal/yakut#65

@pavel-kirienko
Copy link
Member Author

I would like to summon @samcrow for commentary.

@samcrow
Copy link

samcrow commented May 3, 2023

This looks good overall and not very difficult to add to other implementations.

@aentinger
Copy link

May I inquire if there's anything keeping this PR from being merged?

@pavel-kirienko
Copy link
Member Author

pavel-kirienko commented Jul 2, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add string support
3 participants