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

Controller step function provides incorrect parameters with motion multiplier #530

Open
goehle opened this issue Aug 2, 2021 · 3 comments

Comments

@goehle
Copy link

goehle commented Aug 2, 2021

Answer the following questions:

  • what are you trying to do? Run a controller with a motion multiplier and utilize the "t and dt" parameters provided by the step function

  • what is the problem and how can it be recreated? The step function for a controller provides the unmodified time and timestep parameters, even when a motion_mutliplier parameter is present. You can see the issue by printing out the step function parameters and seeing they do not change when the motion multiplier is added. This is caused by this line: https://github.com/gtri/scrimmage/blob/master/src/simcontrol/SimControl.cpp#L1553 where the controller step function is fed t and dt instead of temp_t and motion_dt, even though it is run within the motion multiplier subloop. The fix seems to be just changing those variables in the function call.

  • what scrimmage commit are you on? b696da4

  • describe any changes you made to scrimmage. N/A

  • If you can recreate the issue using only plugins in the scrimmage repository, what mission file are you running? N/A

@kemen209
Copy link

kemen209 commented Jul 13, 2022

I had same doute on this issue. The usage on t_/dt_ changed on commit 'add loop_rate api to plugins', 6e65a69, but the message didn't given out any though on why changed to that way.

src/simcontrol/SimControl.cpp

         // run controllers in a single thread since they are serially connected
         for (EntityPtr &ent : ents_) {
             for (auto c : ent->controllers()) {
-                success &= exec_step(c, [&](auto c){return c->step(temp_t, motion_dt);});
+                success &= exec_step(c, [&](auto c){
+                  return c->step_loop_timer(dt_) ?
+                    c->step(t_, dt_) : true;});
             }
         }

What interesting me is that the SimpleCar model will work smoothly with this change. If i change the t_/dt_ back to temp_t/motion_dt, the SimpleCar model will behave strangely:

(1) desired_speed = 10 m/s, desired_heading = 45 deg, start to turn immediately.
(2) desired_speed = 12 m/s, desired_heading = 45 deg, it will start to turn after 200+ seconds!!!

@kemen209
Copy link

@shaun-d-anderson @esquires Hi, any idea on this?

@kemen209
Copy link

kemen209 commented Jul 14, 2022

What interesting me is that the SimpleCar model will work smoothly with this change. If i change the t_/dt_ back to temp_t/motion_dt, the SimpleCar model will behave strangely:

(1) desired_speed = 10 m/s, desired_heading = 45 deg, start to turn immediately. (2) desired_speed = 12 m/s, desired_heading = 45 deg, it will start to turn after 200+ seconds!!!

After i done some digging in PID controller, this phenomenon will solved by using another set to PID value(lower D factor to its 1/8 would work). So i will stick to the idea of using temp_t/motion_dt is the right way to go.

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

No branches or pull requests

2 participants