-
Notifications
You must be signed in to change notification settings - Fork 290
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
Simulation Reset API #1107
Comments
Is there something missing from the transport interface that is already there? See #203 (comment).
As mentioned on #203 (comment), systems can already detect resets from
+1
That's an interesting idea. As a reference, Gazebo classic offers an optional Reset callback and if it isn't implemented, it's simply not called, and nothing else happens. I wonder if forcebly resetting plugins may have unintended consequences, but I can't think of any right now. |
the most common issue I've seen with osrf/gazebo plugins is storing the most recent time that simulation was run in order to throttle its update rate, then failing to reset that along with the rest of simulation. This can result in plugins failing to update until simulation time reaches the point from which it had previously been reset. |
No, this was just for completeness in documenting the feature. I think that transport is already good to go, but as with @diegoferigo's comment in #203, it makes sense to also have this as part of the C++ API. I would envision this propagating through
I have two primary concerns with relying entirely on
I think adding an interface that system developers could opt-in to would be a safer approach. In terms of (1) it would make it so that there won't be any behavior change if no additional code is added. In the case of (2), it provides an explicit method that will be called in the case of a Reset. It should be very clear to a developer what the expectations are.
I couldn't think of any. In an ideal world, those plugins wouldn't have any internal state (instead storing it in the ECM). In this case, though, I think it's the cleanest way to guarantee that the internal state is reset to some sane initial condition. |
+1
Does that complexity go away if you add a Looking at it from another angle, we currently give developers the flexibility of reacting to a reset at any point in the update loop. A system that updates their internal state in
Is your point that the Assuming we're going with a reset callback, I have some follow up questions:
|
We currently process For me the argument for a
The downside is that we're making a distinction between Reset and Rewind/Time seek. If we ever get to a point where we can go back in time and start the simulation from any arbitrary time instance, then having a |
Ah this was a question that's been in my mind, whether the Another question that's been in my mind is whether you're planning to make the distinction between "time" and "model" resets like Gazebo classic does |
Coming from #1489, My issue feels somewhat duplicated yet it focus on entitymanager mostly. I'm not sure about UpdateInfo since I couldn't find the documentation for it. From what I see, reset is highly possible without the sensor plugins while time is still running. See here, 168909903-ef8d6700-7064-4f22-b05e-a81540cf9c25.mp4This was due to no sensor involves. I can do it all day. The problem is that once you add the sensor in the robot, it automatically adds camera RGB or any sensor that comes first (in this video, camera RGB is the first on the sdf). Once you remove the robot, the sensor is still stored in entity manager. I don't know if See here when you remove the robot, it spammed the error.
This is because it stores the previous RGB camera from the previous robot. So when I load the second robot, it crash. So the question is, which would solve to |
@Kakcalu13 There is a follow up to the first two reset PRs: #1355 This adds the reset capability to sensors and the scenebroadcaster, and may do what you are looking for? |
Oh my! God bless you! Let me try that one! |
Desired behavior
Add the ability for the state of simulation to be reset to what it was at
t=0
. Reset can be called multiple times on the same simulation. A simulation can be stepped or otherwise run after being reset.This feature should provide a relatively-optimized way of resetting simulation to a particular state without incurring unnecessary overhead of reloading and re-parsing assets.
Alternatives considered
Implementation Outline
The implementation should happen in the following general phases:
ign-transport
Reset
functionality.The Reset interface shall be opt-in and not break any existing System plugin implementations. In the case that the Reset interface isn't implemented, the entire system should be destructed and constructed during the Reset phase.
The text was updated successfully, but these errors were encountered: