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

Refactor ROS 2 sensor components events management #482

Merged
merged 45 commits into from
Sep 13, 2023
Merged

Refactor ROS 2 sensor components events management #482

merged 45 commits into from
Sep 13, 2023

Conversation

PawelLiberadzki
Copy link
Contributor

This is a proposition of universal sensor events management, that solves issue #295.

I introduce event source interface, and two sample implementations of it - TickBasedSource and PhysicsBasedSource. They are used through event source adapter in a new sensor component base class ROS2SensorComponentBase (previously ROS2SensorComponent).

@PawelLiberadzki
Add SensorEventSource template.
c14d0fe 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Remove reflection from SensorEventSource.
76c2a18 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Fix cmake for SensorEventSource.
018a2f6 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Add Activate/Deactivate interface to SensorEventSource. Remove not ne…
d3af51c 
…cessary includes. Update documentation.

Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Add TickBasedSource implementation of SensorEventSource base.
2fcb7d5 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Update SensorEventSource documentation.
a666183 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Update documentation and headers in TickBasedSource.
d014f77 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Add EventSourceAdapter.
4221f0d 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Add basic reflection for EventSourceAdapter.
938cd60 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Add reflection for event source inside EventSourceAdapter. Update Eve…
0e1ec96 
…ntSourceAdapter documentation.

Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Add template argument type check for EventSourceAdapter.
dbaf52c 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Add basic implementation for PhysicsBasedSource.
3907bde 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Fix physics based event initialization. Update includes in PhysicsBas…
495110a 
…edSource.

Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Make minor documentation / ui adjustments.
0866d41 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Update type of TickBasedSource event. Adjust event source aliases.
c0fe27d 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Add source event and event handler aliases in EventSourceAdapter.
b697b0f 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Add enabled/disabled feature to PhysicsBasedCallback.
569c600 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Add ROS2SensorComponentBase class.
ca5434c 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Fix inheritance issue in ROS2SensorComponentBase.
0ac67e4 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Add serialization for ROS2SensorComponentBase type using TickBasedSou…
44be69f 
…rce.

Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Update ROS2Lidar2DSensorComponent to new event management.
b224793 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Add auto expand attribute to event source adapter reflection.
ecc761b 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Add documentation for ROS2SensorComponentBase.
23c4798 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
adamdbrw
adamdbrw reviewed Aug 30, 2023
View reviewed changes
Copy link
Contributor

@adamdbrw adamdbrw left a comment

Choose a reason for hiding this comment

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

Nice work!

Gems/ROS2/Code/Include/ROS2/Sensor/Events/TickBasedSource.h Outdated Show resolved Hide resolved
Gems/ROS2/Code/Include/ROS2/Sensor/Events/TickBasedSource.h Outdated Show resolved Hide resolved
Gems/ROS2/Code/Include/ROS2/Sensor/Events/TickBasedSource.h Outdated Show resolved Hide resolved
Gems/ROS2/Code/Include/ROS2/Sensor/Events/SensorEventSource.h Outdated Show resolved Hide resolved
Gems/ROS2/Code/Include/ROS2/Sensor/Events/SensorEventSource.h Outdated Show resolved Hide resolved
Gems/ROS2/Code/Source/Lidar/ROS2Lidar2DSensorComponent.cpp Outdated Show resolved Hide resolved
Gems/ROS2/Code/Include/ROS2/Sensor/Events/EventSourceAdapter.h Show resolved Hide resolved
Gems/ROS2/Code/Include/ROS2/Sensor/Events/EventSourceAdapter.h Outdated Show resolved Hide resolved
Gems/ROS2/Code/Include/ROS2/Sensor/Events/EventSourceAdapter.h Show resolved Hide resolved
Gems/ROS2/Code/Source/Sensor/Events/PhysicsBasedSource.cpp Outdated Show resolved Hide resolved
@PawelLiberadzki
Add missing newline at the end of these files.
8f56096 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Make minor fixes from review - documentation and method scope.
908771b 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Add forwarding event parameters in event source adapter.
0544fde 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Update naming in ROS2Lidar2DSensorComponent.
b77bc3a 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Update naming in ROS2SensorComponentBase.
3261b9c 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Update documentation and style for event sources and event source ada…
70a8e0c 
…pter.

Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Update documentation and style in ROS2SensorComponentBase. Make minor…
db8032b 
… adjustment for PhysicsBasedSource.

Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
adamdbrw
adamdbrw approved these changes Sep 4, 2023
View reviewed changes
Copy link
Contributor

@adamdbrw adamdbrw left a comment

Choose a reason for hiding this comment

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

Good work!

Gems/ROS2/Code/Include/ROS2/Sensor/Events/EventSourceAdapter.h Show resolved Hide resolved
Gems/ROS2/Code/Source/Imu/ROS2ImuSensorComponent.cpp Show resolved Hide resolved
Gems/ROS2/Code/Source/Imu/ROS2ImuSensorComponent.cpp Outdated Show resolved Hide resolved
Gems/ROS2/Code/Source/Lidar/ROS2Lidar2DSensorComponent.cpp Outdated Show resolved Hide resolved
@PawelLiberadzki
Fix serialization issues - make sure that prefabs do not have to be u…
fe8416f 
…pdated.

Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Change missing rigid body handling in ROS2ImuSensorComponent.
79b7b3c 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Update handling minimal frequency in EventSourceAdapter.
f4394e2 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki PawelLiberadzki marked this pull request as ready for review September 4, 2023 13:33
@PawelLiberadzki PawelLiberadzki requested review from a team as code owners September 4, 2023 13:33
@PawelLiberadzki
Merge branch 'development' into feature/universal-sensor-events
eeea1b8 
# Conflicts:
#	Gems/ROS2/Code/Source/Lidar/ROS2Lidar2DSensorComponent.cpp
#	Gems/ROS2/Code/Source/Lidar/ROS2LidarSensorComponent.cpp
#	Gems/ROS2/Code/Source/ROS2SystemComponent.cpp
michalpelka
michalpelka reviewed Sep 7, 2023
View reviewed changes
Copy link
Contributor

@michalpelka michalpelka left a comment

Choose a reason for hiding this comment

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

Please make sure that the contribution compiles with non-unity build.

Gems/ROS2/Code/Include/ROS2/Sensor/Events/EventSourceAdapter.h Show resolved Hide resolved
michalpelka
michalpelka reviewed Sep 7, 2023
View reviewed changes
Gems/ROS2/Code/Source/Imu/ROS2ImuSensorComponent.cpp
auto* sceneInterface = AZ::Interface<AzPhysics::SceneInterface>::Get();
const auto gravity = sceneInterface->GetGravity(sceneHandle);
auto* body = sceneInterface->GetSimulatedBodyFromHandle(sceneHandle, m_bodyHandle);
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest multiple assertions here - the code can mistakenly be executed in a different context by mistake (e.g. in Asset Processor) and a good assertion can be helpful.

Copy link
Contributor

Choose a reason for hiding this comment

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

I belive gravity is not needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added an assert on body pointer - this is a good catch. I left sceneInterface as it is - do you maybe think that it is necessary to assert it, when it is a result from:
AZ::InterfaceAzPhysics::SceneInterface::Get()
engine call? Here I assume, that it won't rather fail, and if it will, then we have some critical issue. Is such reasoning not correct?

@PawelLiberadzki
Fix build problems after merge.
5958ff7 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Update lidars implementation after merge.
946860d 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Fix LidarSensorConfiguration setup.
c74173b 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Add assert in ROS2ImuSensorComponent.
ae23802 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@PawelLiberadzki
Update default lidar 2D publisher topic.
33ce98c 
Signed-off-by: Paweł Liberadzki <pawel.liberadzki@robotec.ai>
@michalpelka michalpelka merged commit 04cbbc8 into o3de:development Sep 13, 2023
2 checks passed
@michalpelka michalpelka mentioned this pull request Sep 18, 2023
Merged
michalpelka pushed a commit to RobotecAI/o3de-extras that referenced this pull request Sep 25, 2023
@PawelLiberadzki @michalpelka
Refactor ROS 2 sensor components events management (o3de#482)
90941af 
Universal event management system for sensor components in ROS 2.

---------
Signed-off-by: Paweł Liberadzki <44069826+PawelLiberadzki@users.noreply.github.com >
@michalpelka michalpelka mentioned this pull request Sep 25, 2023
Merged
michalpelka pushed a commit to RobotecAI/o3de-extras that referenced this pull request Sep 25, 2023
@PawelLiberadzki @michalpelka
Refactor ROS 2 sensor components events management (o3de#482)
fcc80ed 
Universal event management system for sensor components in ROS 2.

---------
Signed-off-by: Paweł Liberadzki <44069826+PawelLiberadzki@users.noreply.github.com >
Signed-off-by: Michał Pełka <michal.pelka@robotec.ai>
@Antoni-Robotec Antoni-Robotec mentioned this pull request Sep 26, 2023
Closed
@adamdbrw adamdbrw mentioned this pull request Sep 29, 2023
Closed
michalpelka pushed a commit to RobotecAI/o3de-extras that referenced this pull request Oct 2, 2023
@PawelLiberadzki @michalpelka
Refactor ROS 2 sensor components events management (o3de#482)
2684f2a 
Universal event management system for sensor components in ROS 2.

---------
Signed-off-by: Paweł Liberadzki <44069826+PawelLiberadzki@users.noreply.github.com >
Signed-off-by: Michał Pełka <michal.pelka@robotec.ai>
michalpelka pushed a commit that referenced this pull request Oct 3, 2023
@PawelLiberadzki @michalpelka
Refactor ROS 2 sensor components events management (#482)
ede0463 
Universal event management system for sensor components in ROS 2.

---------
Signed-off-by: Paweł Liberadzki <44069826+PawelLiberadzki@users.noreply.github.com >
Signed-off-by: Michał Pełka <michal.pelka@robotec.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michalpelka michalpelka michalpelka approved these changes

@adamdbrw adamdbrw adamdbrw approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@PawelLiberadzki @michalpelka @adamdbrw