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.

Sign up for GitHub

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

Jump to bottom

Fix yaserde deserialization of Joint limits #74

Merged
merged 3 commits into from May 10, 2023
Merged

Fix yaserde deserialization of Joint limits #74

merged 3 commits into from May 10, 2023

Conversation

patrvanh
Copy link
Contributor

@patrvanh patrvanh commented May 3, 2023

This fixes the bug described in this #64 comment.

@patrvanh patrvanh mentioned this pull request May 3, 2023
Closed
@luca-della-vedova
Copy link
Contributor

Beat me to it, thanks! Yep yaserde needs explicit tagging for fields that are stored in attributes and will not automatically work for both attributes and child elements as serde-xml-rs used to do. There might be more lurking in code that was not covered by test

@taiki-e
Copy link
Contributor

taiki-e commented May 8, 2023

Thanks for the PR! I have looked at other structs, and it appears that the Mimic, SafetyController, and Dynamics's fields are also all attributes: https://wiki.ros.org/urdf/XML/joint

Could you also update them?

@luca-della-vedova luca-della-vedova mentioned this pull request May 10, 2023
Merged
taiki-e
taiki-e reviewed May 10, 2023
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
@taiki-e taiki-e merged commit e1c1afe into openrr:main May 10, 2023
2 checks passed
@taiki-e taiki-e mentioned this pull request May 10, 2023
Merged
@taiki-e taiki-e mentioned this pull request Jan 9, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@luca-della-vedova luca-della-vedova luca-della-vedova approved these changes

@taiki-e taiki-e taiki-e approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@patrvanh @luca-della-vedova @taiki-e