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

feat(components): get clproto message type from attribute #175

Merged
merged 2 commits into from
Dec 8, 2024

Conversation

domire8
Copy link
Member

@domire8 domire8 commented Dec 5, 2024

Description

One thing that has come to my attention with the recent documentation efforts and webinars is that the fact that we require the user to know the mapping between state representation type and the clproto message type when adding an output in python is a bit too much. I propose here in a non breaking fashion to remedy that by getting the clproto message type from the type of the class attribute. The functionality stays exactly the same (e.g. existing code still works and if a user wants to publish a CartesianPose as a CARTESIAN_STATE_MESSAGE, that is still possible), but for the simple case of the 1:1 mapping, the clproto message type is not required anymore to add an output.

Review guidelines

Estimated Time of Review: 5 minutes

Checklist before merging:

  • Confirm that the relevant changelog(s) are up-to-date in case of any user-facing changes

bpapaspyros
bpapaspyros previously approved these changes Dec 5, 2024
Copy link
Member

@bpapaspyros bpapaspyros left a comment

Choose a reason for hiding this comment

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

great idea, looks good to me. I would probably take a few moments to test, but I from what I see the tests cover it, so go for it if you are satisfied with it (after the CHANGELOG update).

Copy link
Member

@eeberhard eeberhard left a comment

Choose a reason for hiding this comment

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

Very clever, nice idea! This will be convenient for users.

One small discussion point and needs changelog of course.

@domire8 domire8 changed the title feat: get clproto message type from attribute feat(components): get clproto message type from attribute Dec 8, 2024
@domire8 domire8 merged commit dec18cd into main Dec 8, 2024
4 checks passed
@domire8 domire8 deleted the feat/py-clproto branch December 8, 2024 15:47
@github-actions github-actions bot locked and limited conversation to collaborators Dec 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants