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

remap char and byte to int8_t and uint8_t #130

Closed
wants to merge 1 commit into from

Conversation

mikaelarguedas
Copy link
Member

@mikaelarguedas mikaelarguedas commented May 1, 2017

This is a draft of what deprecating char and byte would look like.

What could be done on top: On the python side we could relax the constraint by allowing users to pass builtin.bytes of length 1 and make the conversion in the setter functions

Connects to ros2/rosidl#190

@mikaelarguedas mikaelarguedas added in progress Actively being worked on (Kanban column) in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels May 1, 2017
@mikaelarguedas mikaelarguedas self-assigned this May 2, 2017
Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

lgtm, the rationale makes sense

We may need to (re)consider how we map int types of various sizes to types in Python. Int is fine for now, but in ROS 1 there is an option to use numpy and map them to more efficient types.

I only bring this up because if you imagine transmitting image data, defined as a uint8[] in the message file would become a list of Int in Python, which is quite a bit larger. If we used numpy we could map the [u]int{8,16,32,64} types to their exact size. But even without numpy perhaps there should be a special exception for uint8[] being mapped to bytes or a bytearray in Python (since they are between 0 and 256 too):

Return a new “bytes” object, which is an immutable sequence of integers in the range 0 <= x < 256.

--
https://docs.python.org/3.1/library/functions.html#bytes

Copy link
Member

@dirk-thomas dirk-thomas left a comment

Choose a reason for hiding this comment

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

When I am looking at this from the point of view of the user I see the following case. A user wants to send and receive a sequence of bytes in Python. So I would expect the Python API to write bytes and read bytes. That type is the user supposed to use in the .msg file? I would think byte[] would be the natural choice (which would probably need an update in rosidl_generator_py).

Until this is being figured out I would rather not merge this change,

@mikaelarguedas mikaelarguedas added in progress Actively being worked on (Kanban column) and removed in review Waiting for review (Kanban column) labels May 10, 2017
@dirk-thomas dirk-thomas mentioned this pull request Nov 1, 2017
@dirk-thomas
Copy link
Member

This has been addressed differently in the meantime: see https://github.com/ros2/design/pull/213/files#diff-0307e01fdcc93b6ad5b2241e0bde127fR284

@dirk-thomas dirk-thomas removed the in progress Actively being worked on (Kanban column) label Mar 28, 2019
@dirk-thomas dirk-thomas deleted the deprecate_byte_char branch March 28, 2019 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants