Skip to content

Conversation

saikishor
Copy link
Member

@saikishor saikishor added the backport-jazzy Triggers PR backport to ROS 2 jazzy. label Jun 30, 2025
Copy link
Contributor

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

LGTM, should we also add a test for that?

Copy link
Member

@destogl destogl left a comment

Choose a reason for hiding this comment

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

I was thinking to approach this in another way. I think we should have per default conversion between double and other types (if possible). This will help us with the reusability of HW interface and controllers. We had similar issues in CAN implementation and solved it with memcpy — there we have uint32_t variables for “transport storage”. I know this is not beautiful, but it is extremely useful. We should give a warning once per interface that we are handing the type using double but it is a compatible type and then just do it. Otherwise, I am afraid that we will end up in the endless if - else if loops and having many issues with the controllers — HW incompatibilities, even though they could work with the proposed type support.

What I want to say, we should add this casting to the handles and not in the controllers.

@christophfroehlich christophfroehlich marked this pull request as draft August 29, 2025 11:18
Copy link
Contributor

This PR is stale because it has been open for 45 days with no activity. Please tag a maintainer for help on completing this PR, or close it if you think it has become obsolete.

@github-actions github-actions bot added the stale label Oct 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-jazzy Triggers PR backport to ROS 2 jazzy. stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants