-
Notifications
You must be signed in to change notification settings - Fork 277
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
ForceTorque system: write WrenchMeasured to ECM #2494
Conversation
Specifying an integer value in a <gz:system_priority> tag for a system will control the order in which System PreUpdate and Update callbacks are executed, with lower values executing first. The PriorityPrinter example plugin is added to illustrate the feature. Signed-off-by: Steve Peters <[email protected]>
Set gz:system_priority to positive value for force_torque system in example and test worlds. Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
…rcetorque_write_ecm
Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
Currently the system skips the sensor update step when there are no gz-transport subscribers, but to support writing to the ECM, the system should always update when there is new sensor data. Signed-off-by: Steve Peters <[email protected]>
Add ModelByName and ModelCount APIs. These are needed for the updated ForceTorque test. Signed-off-by: Steve Peters <[email protected]>
* Alphabetize headers * Rename test using gz-transport topics * Add const to some local variables Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
…rcetorque_write_ecm
gazebosim/gz-sensors#449 has been merged and nightlies rebuilt, rerunning CI now |
// note: gz-sensors does its own throttling. Here the check is mainly | ||
// to avoid doing work in the ForceTorquePrivate::Update function | ||
bool needsUpdate = false; | ||
for (const auto &[sensorEntity, sensor] : this->dataPtr->entitySensorMap) | ||
{ | ||
if (sensor->NextDataUpdateTime() <= _info.simTime && | ||
sensor->HasConnections()) | ||
if (sensor->NextDataUpdateTime() <= _info.simTime) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it make sense for users to indicate that they want data to be written to ECM by creating a components::WrenchMeasured
component? here we would just check for the presence of that component, e.g.
if (sensor->NextDataUpdateTime() <= _info.simTime &&
(sensor->HasConnections() ||
_ecm.Component<components::WrenchMeasured>(sensorEntity) != nullptr))
Probably not going to make much perf difference for this system since the sensor Update
call should be relatively fast and same for writing force torque values to ECM. It would be nice to keep things consistent if we do decide to port this functionality to other sensors, e.g. contact or rendering sensors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooh, I like that. Let me give that a try
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This allows users to enable writing the sensor data to the ECM by creating the component. Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
I noticed a python test failing in #2500 and realized I didn't set priority on its test file, so I've done so in ba5408f I'm guessing it will fail, since the python test implicitly assumes that its |
The test's PreUpdate callback assumes that it executes after the ForceTorque::PreUpdate, so just move it to Update to gurantee it. Also fix a spelling error in the callback variable names. Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
the test should be fixed by 1f76c6a, though it's hard for me to run these tests on my machine, so I'll wait for CI to confirm |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2494 +/- ##
==========================================
+ Coverage 65.95% 66.07% +0.12%
==========================================
Files 327 329 +2
Lines 31319 31801 +482
==========================================
+ Hits 20655 21012 +357
- Misses 10664 10789 +125 ☔ View full report in Codecov by Sentry. |
looks good to me. I noticed some windows tests were failing. I think they are flaky as I've seen a couple of them fail before. I just triggered another build to double check. |
The system priority for the force-torque system was explicitly set using XML tags in several test worlds in #2494. These XML tags are no longer needed since #2500 was merged, so remove them now. Signed-off-by: Steve Peters <[email protected]>
Backport nested model accessor APIs from #2494. Signed-off-by: Steve Peters <[email protected]>
The system priority for the force-torque system was explicitly set using XML tags in several test worlds in #2494. These XML tags are no longer needed since #2500 was merged, so remove them now. Also fix some outdated comments Signed-off-by: Steve Peters <[email protected]>
* Model.hh: add ModelByName and ModelCount APIs Backport nested model accessor APIs from #2494. * Add WrenchMeasured component * Physics.cc: fix typo in comment * sensor_TEST.py: move PreUpdate callback to Update The test's PreUpdate callback assumes that it executes after the ForceTorque::PreUpdate, so just move it to Update to gurantee it. Also fix a spelling error in the callback variable names. * fix spelling in on_post_update_cb * backport minor fixes to force-torque test * Include System.hh from some systems, add comment Signed-off-by: Steve Peters <[email protected]>
🎉 New feature
Part of #2268 and #2391, requires gazebosim/gz-sensors#449
and targets the branch for #2487Summary
This adds a new
WrenchMeasured
component and writes to it on the force-torque sensor entities in the ECM. The ForceTorque sensor configuration is changed to call sensor updates even when the gz-transport topic has no subscribers, so that the latest wrench data will be in the ECM. This also addsModel::ModelCount
andModel::ModelByName
APIs togz/sim/Model.hh
to support the new test.Test it
Build from source and run
ctest -R INTEGRATION_force_torque_system
Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-by
messages.