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

Refactor ROS2Sensor component data acquisition loops #295

Closed
adamdbrw opened this issue Apr 24, 2023 · 3 comments
Closed

Refactor ROS2Sensor component data acquisition loops #295

adamdbrw opened this issue Apr 24, 2023 · 3 comments
Assignees
Labels
feature/robotics This item is related to robotics. kind/enhancement Enhancement to an existing feature. priority/major Major priority. Work that should be handled after all blocking and critical work is done. sig/simulation Categorizes an issue or PR as relevant to SIG Simulation triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@adamdbrw
Copy link
Contributor

Is your feature request related to a problem? Please describe.
This is an enhancement to the architecture of current data acquisition mechanism in ROS2Sensor component as introduced with #254.

Describe the solution you'd like
I would like to increase encapsulation and reduce the amount of code that the implementer of ROS2Sensor subclass needs to be aware of in order to succeed. This involves replacing inheritance with composition where needed and implementing NVI for the loops if applicable.

Additional context
The dual mechanism is here because one (onTick()) is simpler and refers to Engine updates, and the second one (physics events) provides a higher-frequency, less-jittery data streams important for sensors such as IMU and Odometry.

@adamdbrw adamdbrw added kind/enhancement Enhancement to an existing feature. feature/robotics This item is related to robotics. labels Apr 24, 2023
@byrcolin byrcolin added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. sig/simulation Categorizes an issue or PR as relevant to SIG Simulation labels Apr 25, 2023
AMZN-alexpete pushed a commit that referenced this issue May 3, 2023
Changed way in which URDF is parsed:
 - entities are created without hierarchy at first
 - entities transformations are set independent of joint
 - Widgets contain log of created entities and joints
 - Fixed crash in ColliderMaker, that was caused by a non-existing mesh
   collider

Co-authored-by: Adam Dabrowski <adam.dabrowski@robotec.ai>
Signed-off-by: Michał Pełka <michalpelka@gmail.com>
@lgleim lgleim added triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/major Major priority. Work that should be handled after all blocking and critical work is done. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels May 11, 2023
@Antoni-Robotec
Copy link
Contributor

The sensors update mechanism is similar to the ones in ROS Publishers, would be best to unify.

@adamdbrw
Copy link
Contributor Author

This means, when working on this task, also look at non-sensor publishers such as JointState.

@PawelLiberadzki
Copy link
Contributor

Resolved with #526.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/robotics This item is related to robotics. kind/enhancement Enhancement to an existing feature. priority/major Major priority. Work that should be handled after all blocking and critical work is done. sig/simulation Categorizes an issue or PR as relevant to SIG Simulation triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

5 participants