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

Adds time interpolation to the Science Sensors #210

Merged
merged 5 commits into from
Jun 15, 2022

Conversation

arjo129
Copy link
Member

@arjo129 arjo129 commented May 13, 2022

As said above. There are several major changes in this PR.

  • Move all the logic for lookup in the science sensor to a separate function ScienceSensorsSystemPrivate::InterpolateInTime().
  • Add time interpolation.
  • Add the ability to load files via a topic instead of hard-coded into the SDFormat world.
  • Add unit tests for time interpolation.

This PR moves all the logic for lookup in the science sensor to a separate function `ScienceSensorsSystemPrivate::InterpolateInTime()`. It also adds time based interpolation to the science sensor.

Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129 arjo129 requested review from braanan and hidmic May 18, 2022 01:42
@caguero caguero mentioned this pull request May 18, 2022
39 tasks
sphericalDepthCorrected, simTimeSeconds, this->dataPtr->eastCurrentArr);


ignition::math::Vector3d current(nCurr, eCurr, 0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@arjo129 meta: there's a subtle change in behavior here, now nCurr or eCurr could be NaN even if the other isn't.

lrauv_ignition_plugins/src/ScienceSensorsSystem.cc Outdated Show resolved Hide resolved
lrauv_ignition_plugins/src/ScienceSensorsSystem.cc Outdated Show resolved Hide resolved
lrauv_ignition_plugins/src/ScienceSensorsSystem.cc Outdated Show resolved Hide resolved
}
EXPECT_TRUE(spawnedAllVehicles);

fixture->Server()->Run(true, 1000, false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@arjo129 meta: there are a number of tools in lrauv_system_tests to reduce duplication, prevent races, reduce total run time, and improve readability across tests. I'd be inclined to use them if time allows. test_spawn.cc is one good example.

Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Copy link
Collaborator

@hidmic hidmic left a comment

Choose a reason for hiding this comment

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

LGTM -- pending comments are not blockers.

Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129 arjo129 merged commit a78b6c5 into main Jun 15, 2022
@arjo129 arjo129 deleted the arjo/feat/timeInterpolation branch June 15, 2022 05:16
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.

2 participants