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

Add Angular Accelerometer and Linear Velocity Sensor Interfaces to the Sensor Remapper #3149

Merged
merged 17 commits into from
Dec 5, 2024
Merged

Add Angular Accelerometer and Linear Velocity Sensor Interfaces to the Sensor Remapper #3149

merged 17 commits into from
Dec 5, 2024

Conversation

nicktrem
Copy link
Contributor

  • This PR will allow the angular acceleration and linear velocity measurements measured by a particular sensor to be extracted and used via the sensor remapper.

@update-docs Update Docs
Copy link

update-docs bot commented Nov 13, 2024

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would update the release notes by adding a file in doc/release/<target_branch>, based on your changes.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@nicktrem nicktrem mentioned this pull request Nov 13, 2024
Merged
@traversaro
Copy link
Member

@randaz81 this is a contribution from an AMI team member, I will iterate on with with @nicktrem, and when it is ready for review I will let you know, thanks!

@traversaro
Copy link
Member

@nicktrem I see that there are some conflicts, probably you can look into solving them. For the conflict in the autogenerated files, it is probably easier if you just regenerated them once you solved the other conflicts.

randaz81
randaz81 reviewed Nov 14, 2024
View reviewed changes
cmake/YarpIDL.cmake Outdated
@@ -193,7 +193,7 @@ endfunction()
function(YARP_IDL_TO_DIR)

# Flag to control whether IDL generation is allowed.
option(ALLOW_IDL_GENERATION "Allow YARP to (re)build IDL files as needed" OFF)
option(ALLOW_IDL_GENERATION "Allow YARP to (re)build IDL files as needed" ON)
Copy link
Member

Choose a reason for hiding this comment

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

This value should not be modified

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 3c5625e

@randaz81
Copy link
Member

Is this PR complete? @nicktrem @traversaro

traversaro
traversaro reviewed Nov 25, 2024
View reviewed changes
src/devices/networkWrappers/multipleanalogsensorsserver/MultipleAnalogSensorsServer.cpp Outdated
Comment on lines 28 to 37
ThreeAxisAngularAccelerometers=2,
ThreeAxisMagnetometers=3,
OrientationSensors=4,
TemperatureSensors=5,
SixAxisForceTorqueSensors=6,
ContactLoadCellArrays=7,
EncoderArrays=8,
SkinPatches=9,
PositionSensors=10,
LinearVelocitySensors=11
Copy link
Member

Choose a reason for hiding this comment

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

Can we just add the new value at the end for avoiding changing the value of the constants unless strictly necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 8ec2a36

@traversaro
Copy link
Member

My only blocking comment is https://github.com/robotology/yarp/pull/3149/files#r1856392918 . Beside that, this introduces a communication incompatibility between YARP 3.10 and YARP master for multipleanalogsensorsserver and multipleanalogsensorsclient, but I guess this is kind of expected and documented, so once https://github.com/robotology/yarp/pull/3149/files#r1856392918 is addressed this is ready for be reviewed for me.

@nicktrem
Copy link
Contributor Author

The development of this branch has been completed. I have marked this PR as ready to review.

@randaz81 @traversaro please let me know if there are any additional requests, Thanks!

@nicktrem nicktrem marked this pull request as ready for review November 26, 2024 13:50
@nicktrem nicktrem requested a review from Nicogene as a code owner November 26, 2024 13:50
@xela-95 xela-95 mentioned this pull request Dec 3, 2024
Open
@randaz81 randaz81 added this to the YARP 3.11.0 milestone Dec 3, 2024
@randaz81 randaz81 self-assigned this Dec 3, 2024
nicktrem and others added 5 commits December 5, 2024 09:28
@nicktrem @traversaro
Corrected units in comment
f2a0b04 
Co-authored-by: Silvio Traversaro <silvio@traversaro.it>
@nicktrem @traversaro
Corrected units in comment for angular acceleration
b1ae3df 
Co-authored-by: Silvio Traversaro <silvio@traversaro.it>
@nicktrem @traversaro
Corrected units in comment for angular acceleration
cb50022 
Co-authored-by: Silvio Traversaro <silvio@traversaro.it>
@nicktrem
Merge branch 'master' into addAngAccLinVelSensor
f0170c0
@nicktrem
Applied clang format on new merged file for CI
d496ecd
@nicktrem nicktrem deployed to code-analysis December 5, 2024 09:28 — with GitHub Actions Active
@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Dec 5, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
33.1% Coverage on New Code (required ≥ 80%)
29.2% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@traversaro
Copy link
Member

@randaz81 for me the PR is good to go.

@randaz81 randaz81 merged commit 6b4bb23 into robotology:master Dec 5, 2024
45 of 50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@randaz81 randaz81 randaz81 left review comments

@traversaro traversaro traversaro approved these changes

@Nicogene Nicogene Awaiting requested review from Nicogene Nicogene is a code owner

Assignees

@randaz81 randaz81

Labels
Component: Devices
Projects
None yet
Milestone
YARP 3.11.0
Development

Successfully merging this pull request may close these issues.
4 participants
@nicktrem @CLAassistant @traversaro @randaz81