Skip to content

Add API for enabling / disabling IMU orientation#142

Merged
chapulina merged 3 commits intoign-sensors4from
imu_orientation
Jul 10, 2021
Merged

Add API for enabling / disabling IMU orientation#142
chapulina merged 3 commits intoign-sensors4from
imu_orientation

Conversation

@iche033
Copy link
Contributor

@iche033 iche033 commented Jul 1, 2021

Signed-off-by: Ian Chen ichen@osrfoundation.org

🎉 New feature

Not all IMUs produce orientation estimates. This PR adds a new API to the IMU sensor class to allow users to disable computing and publishing orientation values.

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 <ichen@osrfoundation.org>
@iche033 iche033 requested a review from mjcarroll July 1, 2021 23:57
@github-actions github-actions bot added the 🔮 dome Ignition Dome label Jul 1, 2021
@codecov
Copy link

codecov bot commented Jul 2, 2021

Codecov Report

Merging #142 (42522e8) into ign-sensors4 (e9aba3d) will increase coverage by 0.05%.
The diff coverage is 100.00%.

❗ Current head 42522e8 differs from pull request most recent head 43c4050. Consider uploading reports for the commit 43c4050 to get more accurate results
Impacted file tree graph

@@               Coverage Diff                @@
##           ign-sensors4     #142      +/-   ##
================================================
+ Coverage         76.10%   76.16%   +0.05%     
================================================
  Files                23       23              
  Lines              2390     2396       +6     
================================================
+ Hits               1819     1825       +6     
  Misses              571      571              
Impacted Files Coverage Δ
src/ImuSensor.cc 89.09% <100.00%> (+0.62%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e9aba3d...43c4050. Read the comment docs.

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

LGTM, just a nit about style.

iche033 added 2 commits July 1, 2021 17:51
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

A minor style issue, but otherwise LGTM

// orientation should still be the previous value because it is not being
// updated.
EXPECT_EQ(newOrientValue, sensor->Orientation());

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

@chapulina chapulina merged commit 95bff1b into ign-sensors4 Jul 10, 2021
@chapulina chapulina deleted the imu_orientation branch July 10, 2021 02:06
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.

3 participants