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

flir_ptu simulation #18

Open
guihomework opened this issue Jun 2, 2015 · 12 comments
Open

flir_ptu simulation #18

guihomework opened this issue Jun 2, 2015 · 12 comments

Comments

@guihomework
Copy link

Dear maintainer,

I am interested by a flir_ptu simulation. I found that some discussions occurred partially in that direction
ros-controls/ros_controllers#112 but I could not find a flir ptu simulation.

I dediced to develop one. I created two packages

  • flir_ptu_simulation that extends flir_ptu_description on all the gazebo and transmission tags required such as ros_control plugins. In this way controllers can be plugged in, especially the head pointing controllers existing for ros_control probably.
  • flir_ptu_controllers that provides a ros_control controller taking JointStates as cmd as in the real driver (does not exist in ros_controllers)

I had to modify flir_ptu_description to include inertia values (estimated ones) in order for gazebo to accept spawning the ptu model, and also added collision elements in the urdf, but that should not affect other users too much.

I got a first working system, but have a few questions before sharing that upstream as a PR

  • First of all are you interested ?
  • Should the controllers provide exactly the same response as the driver ? for instance I can have the real velocity published in the simulated /joint_states (using a joint_state_controllers) driver and not the max velocity setpoint as in the driver? Or should the provided joint_states look exactly as in real world with constant velocity equal to max velocity setpoint ?

best

@mikepurvis
Copy link
Member

Very much interested!

The simulator should strive to be a drop-in replacement for the real device. However, note that this driver code is extremely long in the tooth. In cases where modern ros_control conventions disagree with how this driver works, the driver should be changed to match them, not the other way around. It would be great to move the driver toward using JointStateController rather than its own joint_state publishing.

Unfortunately, I just don't have a lot of time available to commit to maintaining this code— I'm happy to review, but that's the extent of my availability on it. Would you be interested in becoming a maintainer for the flir_ptu repo and associated packages?

@guihomework
Copy link
Author

Dear @mikepurvis

We are not strong users of this camera, I discovered we had one in our lab a 2 months ago and because there were ros driver out there, we used it for an experiment. So I am not interested in maintaining it but willing to share our own work.

Because I have no idea what this camera can actually really provide in terms of control and velocity reading, I tried to replicate what I could see with the existing driver. From what I noticed, the internal controller ramps up to the provided velocity and slows down towards the end. It provides the current position and current max velocity. We cannot read the current velocity from the joint_states.
To the best of my knowledge, there is no controller like this in ros_control (max velocity profile). Most of the time, a trajectory controller would command the low-level controllable variable with position,velocity or effort to follow a nice trajectoty profile. Unfortunately, these trajectory controllers all work on time constraints not on velocity constraints.
This choice helps all the joints to finish at the same desired time_from_start with velocity being adjusted toward this goal.
I re-implemented a rather simple velocity ramp-up and ramp-down in a PID position controller with internal velocity controller ( i gain only controller ) to limit the velocity to max_velocity.
I need a few more tests and will provide one or more pull requests then.

@gtinchev
Copy link

@guihomework I am currently looking at implementing simple PID position controller with a FLIR PTU d48 plugin in Gazebo.
I couldn't find the PR you mentioned.
Do you mind giving giving an update?

Regards,
Georgi

@guihomework
Copy link
Author

@gtinchev We never did the PR due to no time for testing deeper on that (my colleague using this camera left our lab) but the latest code I have is there
https://github.com/ubi-agni/flir_ptu/tree/indigo-devel
I am willing to have our code merged if it is useful and tested.

If you have issues testing it, or if it works nice for you, give feedback here and I might then create the PR sooner. I could also fix things if not too time consuming.

We even have support for RS485 and more control in another branch which I did not review yet for the same reason as above mentioned
https://github.com/ubi-agni/flir_ptu/commits/driver-devel

We will use this code in the future (the ptu is installed in the lab) but this development is kind of paused until needed again (maybe in 2 or 3 months)

@gtinchev
Copy link

@guihomework works like a charm! I added D48 support, just need to tune the gains and will do a PR to your fork so you can then forward it to the original driver. Good work!

@rarmstrong22
Copy link

@gtinchev @guihomework Do you do anything special to get the D46 simulation to work?

If I do roslaunch flir_ptu_simulation.launch, the model spawns in Gazebo and I don't see any error messages, but I can't get the unit to move using flir_ptu_driver/scripts/cmd_angles. Looking in Gazebo, I notice that ptu_mount_link and ptu_pan_link are not present. Strangely, ptu_pan_link and ptu_mount_link both show up In RViz, but in wonky positions. RViz is also missing transforms for some of the links.
Any advice is greatly appreciated.

@gtinchev
Copy link

Try checking the channels - whether you can get the joint angles by subscribing from terminal.

@guihomework
Copy link
Author

Hi,

while looking at our code and rebasing I found out one other community member (@athackst) redid (after me) part of the simulation stuff and directly did a PR #21, without commenting on/referring to the original issue #18. She had the chance that @mikepurvis was available at that time to get a merge the same day.

I had an overlook on PR #21 and it seems to use default trajectory controllers from ros_control (#21 (comment)) which do not have the same interface as the driver (JointStates) and do not do velocity profile according to a max velocity but to a given trajectory time. This is different than the driver (was different at the time of my solution and I did not see driver changes)

It is a pity things were not coordinated to get a nicer simulation, closer to the real driver. Now, I suppose the only solution is to completely compare the two solutions and discuss what is desirable (in terms of controller behaviour) for the simulation.

@rarmstrong22 I think I would rather put my time on a new PR than trying to rebase mine for kinetic and understand your problem. I would try to also include the changes of @gtinchev directly if it makes sense.

I hope I can find some time to do it next week but would like to have a point of view of a maintainer @TheDash before I schedule this work.

thanks

@rarmstrong22
Copy link

@guihomework Thanks for taking a look. In the end, I decided to write my own PTU simulation, as a learning exercise. We expect to have a FLIR E46 here soon, so I'll be watching the coming commits with interest.

@gtinchev Thanks for the advice.

@TheDash
Copy link
Contributor

TheDash commented Apr 21, 2017

Hi,

it was missing inertial joints. Gazebo should work now.

@TheDash
Copy link
Contributor

TheDash commented Apr 21, 2017

"It is a pity things were not coordinated to get a nicer simulation, closer to the real driver. Now, I suppose the only solution is to completely compare the two solutions and discuss what is desirable (in terms of controller behaviour) for the simulation."

A PR can be made that adds the controller interface for JointStates. ROS Control supports this.

If I do roslaunch flir_ptu_simulation.launch, the model spawns in Gazebo and I don't see any error messages, but I can't get the unit to move using flir_ptu_driver/scripts/cmd_angles. Looking in Gazebo, I notice that ptu_mount_link and ptu_pan_link are not present. Strangely, ptu_pan_link and ptu_mount_link both show up In RViz, but in wonky positions. RViz is also missing transforms for some of the links.
Any advice is greatly appreciated.

It's quite possible the inertial links are missing for those that aren't appearing in Gazebo, and thus gazebo collapses them.

@guihomework
Copy link
Author

@TheDash I can indeed create a PR with the controller I created, but it is more than just a new input type, it does the ramping up of the velocity until max velocity as does the real driver.
If this is not desirable, because only time based trajectory interpolation is wanted (and is already implemented by ros_control), than I would not spend the time to add this unwanted type of controller.

Also the joint_state_controller publishing the joint_states might not produce similar output to the real driver. In the real driver only max_velocity is published back (to know what value of max velocity the driver considers for motion execution). The joint_state_controller would produce the velocity value that gazebo passes or maybe even 0 (because a position_interface is used), not sure.

As a summary, I am ok to free some time of a busy schedule to work again on my more accurate simulation if this behaviour is what maintainers want in the end. I suppose one could have 2 options of controllers, the existing one or a more accurate one, but spending time on something not used is wasting time (unless you want to learn, which is not my case here). I hope you understand my point of view

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

5 participants