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 Angular Acceleration and Linear Velocity Values #48

Merged
merged 11 commits into from
Dec 13, 2024

Conversation

nicktrem
Copy link

Upon the acceptance of the corresponding YARP pull request, this pull request will allow access to the measured Angular Acceleration and Linear Velocity values from supported Realsense devices.

Copy link
Member

@Nicogene Nicogene left a comment

Choose a reason for hiding this comment

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

Some minor comments, I would add the required YARP version that includes the new methods of the MAS interface

(I re-enabled the CI that was disabled for inactivity)

src/devices/realsense2/realsense2Tracking.cpp Outdated Show resolved Hide resolved
src/devices/realsense2/realsense2Tracking.cpp Outdated Show resolved Hide resolved
@nicktrem
Copy link
Author

nicktrem commented Dec 5, 2024

Some minor comments, I would add the required YARP version that includes the new methods of the MAS interface

(I re-enabled the CI that was disabled for inactivity)

@Nicogene This will be done once the YARP version with the changes specified in this PR is released, until then we will wait patiently 😄

@traversaro
Copy link
Member

@nicktrem now that robotology/yarp#3149 has been merged, if you like we can merge this PR in an experimental branch in this repo, if it simplifies the usage downstream. I would just avoid to merge it to master as that branch is meant to be compatible with releases YARP versions.

@nicktrem nicktrem changed the base branch from master to devel December 12, 2024 14:05
@nicktrem
Copy link
Author

@traversaro Sure, we can do that, I just requested that this branch be merged into devel instead of master for now. Once the YARP version gets released with the necessary changes, we can then merge devel into master.

@traversaro
Copy link
Member

Ack! The CI will be broken unless we switch to building YARP from source, but as I do not expect a lot of traffic in the devel branch, I think that is ok. @Nicogene are your comments addressed?

@traversaro traversaro requested a review from Nicogene December 12, 2024 14:10
out[15] = m_last_pose.acceleration.z;
out[16] = m_last_pose.angular_acceleration.x;
out[17] = m_last_pose.angular_acceleration.y;
out[18] = m_last_pose.angular_acceleration.z;
Copy link
Member

Choose a reason for hiding this comment

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

Are you using this port? If not I would just leave it as it was before, as the IAnalogSensor interface is mostly deprecated, and if we add features people may want to use it. @nicktrem

Copy link
Author

Choose a reason for hiding this comment

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

We do not use this port, I agree with you. This has been fixed in cce5f2c

@Nicogene
Copy link
Member

Ack! The CI will be broken unless we switch to building YARP from source, but as I do not expect a lot of traffic in the devel branch, I think that is ok. @Nicogene are your comments addressed?

Yes!

@traversaro traversaro merged commit 5fc2141 into robotology:devel Dec 13, 2024
0 of 2 checks passed
@traversaro
Copy link
Member

Thanks @nicktrem ! Feel free to ping me merge devel in master back in January/February if we have a new YARP release, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants