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

Inconsistent way to trigger sensors in ROS2 Gem #805

Open
michalpelka opened this issue Dec 5, 2024 · 2 comments · May be fixed by #807
Open

Inconsistent way to trigger sensors in ROS2 Gem #805

michalpelka opened this issue Dec 5, 2024 · 2 comments · May be fixed by #807
Labels
kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. sig/simulation Categorizes an issue or PR as relevant to SIG Simulation

Comments

@michalpelka
Copy link
Contributor

michalpelka commented Dec 5, 2024 •
edited
Loading

Describe the bug
ROS2 Gem has two ways to trigger sensors :

  • TickBasedSource
  • PhysicsBasedSource
    Both those wrap OnTickBus and SceneEvents::OnSceneSimulationFinishHandler.
    Those calls implementation of sensor measurements.
    Problem is that both implementations do not respect simulation time
        m_onSceneSimulationEventHandler = AzPhysics::SceneEvents::OnSceneSimulationFinishHandler(
            [this](AzPhysics::SceneHandle sceneHandle, float deltaTime)
            {
                m_sourceEvent.Signal(sceneHandle, deltaTime);
            });

and

    void TickBasedSource::OnTick(float deltaTime, AZ::ScriptTimePoint time)
    {
        m_sourceEvent.Signal(deltaTime, time);
    }

Those delta times are not the same, one is update dt time of PhysX second one it wall time.
It should be consistent and use ROS2 Gem API to get time:

        m_onSceneSimulationEventHandler = AzPhysics::SceneEvents::OnSceneSimulationFinishHandler(
            [this, ros2Interface](AzPhysics::SceneHandle sceneHandle, float deltaTime)
            {
                const auto simulationTime = ros2Interface->GetROSTimestamp();

                float deltaSimTime = ROS2Conversions::GetTimeDifference(m_lastSimulationTime, simulationTime);
                m_sourceEvent.Signal(sceneHandle, deltaTime);
            });

Assets required
N/A

Steps to reproduce

  • Use "SteadyClock":
{
    "O3DE":
    {
        "InputSystem":
        {
            "Mouse":
            {
                "CaptureMouseCursor": false
            }
        },
        "ROS2":
        {
            "SteadyClock" : true,
            "Camera":
            {
                "AllowPipelineModification": true
            }
        }
    }
}
  • Set PhysX engine to be very non-realtime:
    image
  • Add camera and Imu component.
  • Set Imu freq to 100 hz and camera 60 fps
  • Make sure App runs at 60 fps.
    Measure hz of messages using ros2 topic hz:
ros2 topic hz /Camera/camera_image_color --use-sim-time
ros2 topic hz /Cannonball/imu --use-sim-time
ros2 topic hz /Camera/camera_image_color
ros2 topic hz /Cannonball/imu

Expected behavior

topic Hz with --use-sim-time Hz without --use-sim-time
imu 100 6
camera ~60 ~ 3.6

Actual behavior

topic Hz with --use-sim-time Hz without --use-sim-time
imu 100 6
camera 997 60

Screenshots/Video

Peek.2024-12-05.11-17.mp4

Found in Branch
development, main
@michalpelka michalpelka added kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Dec 5, 2024
@michalpelka
Copy link
Contributor Author

Loosely connected to #804
Definitely part of #710

@michalpelka
Copy link
Contributor Author

michalpelka commented Dec 5, 2024 •
edited
Loading

Early implementation : https://github.com/RobotecAI/o3de-extras/tree/mp/adjustments_to_sensor_triggering
Test https://github.com/RobotecAI/o3de/tree/mp/physx_realtime_factor

@michalpelka michalpelka added sig/simulation Categorizes an issue or PR as relevant to SIG Simulation and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Dec 5, 2024
@michalpelka michalpelka linked a pull request Dec 10, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. sig/simulation Categorizes an issue or PR as relevant to SIG Simulation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use ROS 2 clock for sensor triggering RobotecAI/o3de-extras
1 participant
@michalpelka