Simplify /get_scene service #746
Conversation
|
I updated the failing test to use the |
|
my update to the test is a bit flaky on github actions machines 😢 |
Currently to get scene information, a delphyne user must advertise a service that expects a Scene message, then call /get_scene with the name of that service. This can be done with only one service call by returning the Scene message directly from /get_scene. The simulation_runner_test still needs to be updated.
scpeters
force-pushed
the
scpeters/simplify_get_scene
branch
from
33b7ba0
to
4d8147f
Compare
|
rebased and attempting to fix the modified test in 4d8147f. it passes locally for me, so it's a little blind, trying to guess what is going wrong on github actions |
|
all the actions just failed. I've updated the setup-ros tag to use to the latest one (0.1.2) in eae6578 |
|
I had to rerun the gcc actions one time due to other flaky tests, but I think the test I've updated is now working, so this should be good to go as long as scan_build passes |
|
|
||
| scene_request_msg.set_response_topic(service_name); | ||
| // Wait and ensure no steps are taken | ||
| std::this_thread::sleep_for(std::chrono::milliseconds(100)); |
There was a problem hiding this comment.
Can we use timeout variable? Probably a better name can be selected so we reference it by the time it represents rather than the function it has.
There was a problem hiding this comment.
I added waitDuration in f07cb03
Currently to get scene information, a delphyne user must advertise a service that expects a Scene message, then call
/get_scenewith the name of that service. This process is simplified to use only one service call by returning the Scene message directly from/get_scene.The
ProcessSceneRequestmethod is now unused, so it is removed.These changes have broken the
simulation_runner_test, which expects the old behavior of the/get_sceneservice. This test will need to be updated before merging.