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

Update Sensor DOM to Element functions and add tests #757

Merged
merged 6 commits into from
Nov 23, 2021

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Nov 17, 2021

🦟 Bug fix

Fixes #

Summary

Renamed PopulateElement() to ToElement() based on feedback from pull request #755

The PopulateElement functions were added in #753 and have not been released yet so this should not break API/ABI.

Added unit tests.

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

@iche033 iche033 requested a review from nkoenig November 17, 2021 08:47
@github-actions github-actions bot added 🌱 garden Ignition Garden 🏯 fortress Ignition Fortress labels Nov 17, 2021
@iche033 iche033 requested a review from azeey November 17, 2021 08:48
Signed-off-by: Ian Chen <[email protected]>
@codecov-commenter
Copy link

Codecov Report

Merging #757 (e1a826b) into sdf12 (ad00668) will increase coverage by 0.04%.
The diff coverage is 90.00%.

❗ Current head e1a826b differs from pull request most recent head 2ed0597. Consider uploading reports for the commit 2ed0597 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##            sdf12     #757      +/-   ##
==========================================
+ Coverage   88.57%   88.62%   +0.04%     
==========================================
  Files          76       76              
  Lines       11740    11781      +41     
==========================================
+ Hits        10399    10441      +42     
+ Misses       1341     1340       -1     
Impacted Files Coverage Δ
src/Lidar.cc 87.04% <66.66%> (-3.19%) ⬇️
src/Camera.cc 87.17% <84.21%> (+0.47%) ⬆️
src/Sensor.cc 92.01% <93.54%> (+1.45%) ⬆️
src/AirPressure.cc 100.00% <100.00%> (ø)
src/Altimeter.cc 100.00% <100.00%> (ø)
src/ForceTorque.cc 89.93% <100.00%> (+3.56%) ⬆️
src/Imu.cc 100.00% <100.00%> (ø)
src/Magnetometer.cc 100.00% <100.00%> (ø)
src/Noise.cc 95.83% <100.00%> (-0.04%) ⬇️
... and 2 more

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 f2a286c...2ed0597. Read the comment docs.

@nkoenig
Copy link
Contributor

nkoenig commented Nov 22, 2021

@azeey , are you okay with this PR?

Copy link
Collaborator

@azeey azeey left a comment

Choose a reason for hiding this comment

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

LGTM!

@nkoenig nkoenig merged commit cc2d819 into sdf12 Nov 23, 2021
@nkoenig nkoenig deleted the sensor_to_element branch November 23, 2021 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress 🌱 garden Ignition Garden
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants