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 enable_orientation SDF element to imu #651

Merged
merged 3 commits into from
Sep 3, 2021
Merged

Add enable_orientation SDF element to imu #651

merged 3 commits into from
Sep 3, 2021

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Aug 3, 2021

Signed-off-by: Ian Chen [email protected]

🦟 Bug fix

Summary

Add enable_orientation element to SDF spec that allows users to configure whether an IMU sensor will generate and output orientation data

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

Signed-off-by: Ian Chen <[email protected]>
@chapulina chapulina requested a review from scpeters August 23, 2021 18:48
@scpeters
Copy link
Member

the code looks fine; we just have to be careful when adding fields to the spec, both when merging forward, and in terms of back porting to libsdformat6 and libsdformat9

do you have a plan for forward and back-porting?

@iche033
Copy link
Contributor Author

iche033 commented Aug 30, 2021

do you have a plan for forward and back-porting?

We currently don't need this in earlier versions but I can backport it if we want to keep the spec consistent?

What's the process for merging forward? Should I create separate PRs to add the field to the 1.8 (sdf11) and 1.9 (sdf12) specs?

@scpeters
Copy link
Member

scpeters commented Sep 2, 2021

do you have a plan for forward and back-porting?

We currently don't need this in earlier versions but I can backport it if we want to keep the spec consistent?

this would be easier if we kept the spec and parser in separate repositories. we'd probably need to add a patch number to the spec to handle new fields (i.e. 1.6.1). Until then, let's try to keep them consistent

What's the process for merging forward? Should I create separate PRs to add the field to the 1.8 (sdf11) and 1.9 (sdf12) specs?

you can copy them over to the new specs in the merge-forward pull request

@iche033
Copy link
Contributor Author

iche033 commented Sep 3, 2021

sounds good, I merge this first and then start porting to other branches.

@iche033 iche033 merged commit a0bec29 into sdf10 Sep 3, 2021
@iche033 iche033 deleted the imu_orientation branch September 3, 2021 17:51
@scpeters scpeters mentioned this pull request Jan 11, 2022
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔮 dome Ignition Dome
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants