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

Adapt PID controller from Selfie 3.0 #30

Merged
merged 3 commits into from
Dec 2, 2018
Merged

Adapt PID controller from Selfie 3.0 #30

merged 3 commits into from
Dec 2, 2018

Conversation

Goldob
Copy link
Contributor

@Goldob Goldob commented Nov 18, 2018

Resolves #4.

Most of the code has been copied from F1/10 repository, with the most notable difference being added support for two separate offset topics (position and angular). These topics are combined into one, fed into a PID controller and then the resulting effort is translated into steering commands. Automatic rescaling of Ki and Kd parameters based on vehicle speed is also possible.

Everything has been manually tested to work as desired, both on the level of individual nodes and as a complete subsystem. The last thing to do is find optimal parameters for the new car and put them into a single launch file. For now, the controller can be tuned by hand with dynamic reconfiguration after running:

roslaunch selfie_control pid-tuning.launch

The above command will start all the required nodes working in the following layout:

pid-tuning

@Goldob Goldob requested a review from nels3 November 18, 2018 21:48
@Goldob Goldob changed the base branch from master to develop November 18, 2018 21:59
@nels3
Copy link
Member

nels3 commented Nov 18, 2018

  1. Building ok
  2. Does command shouldn't be roslaunch selfie_control pid-tuning.launch? pid.tuning.launch file is inside selfie_control not selfie_launch
  3. Do I need to have another rosrun before this roslaunch? After running command roslaunch after roscore I have one red line like cannot launch node of pid type. After it I have process [...] started with pid [...] and so on.
  4. Should I test here sth?

@Goldob
Copy link
Contributor Author

Goldob commented Nov 19, 2018

Ad 1. You are correct, my mistake. Fixed now 🔧
Ad 2. The launch file should work out of the box, without additional node prerequisites. Please share the whole output you get after running the command.
Ad 3. Mainly sanity checks, which you've already done. I also need you to look over the launch file and individual nodes to understand how they interconnect and if the solution makes sense to you. Suggestions for code improvements are also welcome. I asked you to do the review because you know Python.

And last but not least, confirm if this fulfills the specification from the related issue.

@nels3
Copy link
Member

nels3 commented Nov 19, 2018

Ad 2. There was missing package. I looked into pid-tuning file and there was PID package needed. When I installed it it works (so if somebody do not have this installed should do it).
So now I have all four process started (before only three were starting), but after a couple of second pid_controller has error: process has died and inside log file there is pid parameters. I think it is not sth wrong with code rather with my virtual machine, what do you think? Or maybe with code? I attached log life:
pid_controller-1-stdout.log

@Goldob
Copy link
Contributor Author

Goldob commented Nov 19, 2018

The pid package is specified as dependency in selfie_control, so it should be installed automatically when running the commands from repo's readme.

The process has died thing looks very strange. Perhaps you launched twice and there was a collision between node names? Could you restart everything, invoke roslaunch again and paste its output here?

@nels3
Copy link
Member

nels3 commented Nov 19, 2018

I just have written what I have done and that now process starts.

Still process dies after couple of seconds. I attach log file (new) and the screen from console.
pid_controller-1-stdout.log
screenshot from 2018-11-19 17-19-49

@Goldob
Copy link
Contributor Author

Goldob commented Nov 19, 2018

Seems like this sometimes just happens for some reason. Here is a similar issue with launch file from the official pid package repository. Talking about works on my machine...

Could you run roslaunch pid servo_sim.launch and check if it also crashes?

In the meantime, I will try and dig into that 😬

@nels3
Copy link
Member

nels3 commented Nov 19, 2018

Yeah, same. One process (left wheel pid) died, other works and there are are new windows opened. Same message as in that link, but other process dies after "ROS_INFO prev_time is 0, doing nothing" not before.

@Goldob Goldob merged commit 942f586 into develop Dec 2, 2018
@Goldob Goldob deleted the pid_controller branch December 3, 2018 15:49
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.

None yet

2 participants