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

Disable hover, complete navdata feedback #31

Merged
merged 7 commits into from
Nov 23, 2012
Merged

Disable hover, complete navdata feedback #31

merged 7 commits into from
Nov 23, 2012

Conversation

mikehamer
Copy link
Contributor

@mani-monaj This pull request does a few things:

1. Allow ALL data sent back from the drone to be accessed, rather than the very small subset available through the /ardrone/navdata channel.
To use this, consult the new launch file, you will see many new flags such as enable_navdata_references. These flags control whether the contents of this particular navdata message will be published (if not specified, the topics won't be published). For message contents, consult the new messages in /msg, or the SDK file navdata_common.h. I am using it to receive additional information about the drone's internal controller (navdata_references) for mathematical modeling purposes.

2. Turn off combined navdata messages
By default, the navdata, mag and imu topics will always be published. This can be turned off by setting enable_legacy_navdata to false in the launch file (if not specified, defaults to true).

3. Disable hover mode
As per @parcon 's request, by specifying command_disable_hover=true in the launch file, hover can be disabled (the drone will not try to control to vx=vy=0 when not actively being flown)

4. Always send hover commands (even unchanged ones)
Setting command_always_send in the launch file will force the sending of hover commands. This only has an effect if the hover mode is not disabled.

Regarding testing: I have been flying with change 1 for about a week now and it works as expected.

I flew with change 3 and 4 this afternoon after making the change, and they both worked as expected. Recorded flight data shows that the drone never left flying mode (state==3). Visually, the drone also did not stabilize velocity after commanding to 0 body angle, as is expected when not entering hover.

mani-monaj and others added 6 commits November 12, 2012 12:01
Now only loads the writeable subset of drone parameters
This is an aggressive launch file, you should turn down the
euler_angle_max, control_vz_max and control_yaw if you are not
interested in aggressive flight!
@parcon
Copy link

parcon commented Nov 21, 2012

When running experiments, I've found that it is nice to get the drone in the air using the built in hover mode, then switch to a pure flying mode. I think we should allow for dynamic shifting from hover to flying mode. Maybe a new topic? /ardrone/settings/hover_enable?

I think developing a subtopic concerning some high level decisions that can be changed mid-flight (aka those not set in the launch file) could be a nice place to put things like this going forward.

@mikehamer
Copy link
Contributor Author

That's a good point. Do you envisage a single topic, for example /ardrone/update_configuration which takes a message structured something like:

float32 euler_angle_max
float32 control_vz_max
...etc
bool control_enable_hover

which when received will cause a configuration update? Thinking about it, this shouldn't be too hard to integrate with the current driver if there is interest?

@parcon
Copy link

parcon commented Nov 21, 2012

I've got to think about it further, but we've got a few options:
-The topic method you mentioned (easier, but a lot of data to pass around constantly)
-Individual topics (more code, but cleaner to use in the nodes)
-Services (like the camera toggle)

Either way, I think being able to dynamically change these parameters is important. The service method may be nice for the simple state machine bool variables, where the topic method could be better for the floats.

@mikehamer
Copy link
Contributor Author

I guess the 4th option would be to expose the configuration parameters through a dynamic_reconfigure server.

Regarding setting up individual topics for each configuration parameter, this is actually quite easy. The configuration parameters and all associated loading, setting, etc code are generated through a few lines of C macros (eg ardrone_sdk.cpp:104). Extending this to create an individual topic for each configuration parameter would not be difficult; I'm happy to do this tomorrow if people agree it's the best option? @mani-monaj, @parcon?

The above is true at least for all drone parameters.

For driver parameters such as enabling or disabling hover, I agree perhaps a service call may be the cleanest way, or an empty topic like is used for land/takeoff/reset.

@parcon
Copy link

parcon commented Nov 22, 2012

For ease of implementation I would suggest individual topics. I would be happy to lend my assistance in testing since the code will be quickly generated via the macro.

@mikehamer
Copy link
Contributor Author

Sounds good. It's 1AM here (Zürich, Switzerland), so I'm calling it a night. I'll implement these changes first thing in the morning and post an update here when I'm done.

@mani-monaj
Copy link
Member

In my opinion, the ROS standard way to change the configurations on the fly is by using services. This way GUI can be added to manipulate the parameters using tools like dynamic_reconfigure. The question is how many parameters should be configurable on the fly. So far, we have these service, none of them changing a variable in the parameters section.

I need to take a look into dynamic_reconfigure again, as far as I can remember, the package could magically create appropriate services to reconfigure parameters you specify. The only missing piece is a callback function to re-apply changed parameters.

@parcon
Copy link

parcon commented Nov 22, 2012

Dynamic reconfigure is a good method, although I don't think I've ever changed variables outside of the GUI interface.

@mikehamer
Copy link
Contributor Author

Actually, thinking about it this morning, I think the best option would be simply to have an `/ardrone/update_configuration' topic/service, which just reads the ros parameters from the parameter server and then updates if necessary.

This way we just need to update values on the parameter server and call the topic/service, rather than sending lots of messages and cluttering the service/topic namespace with a service/topic for every configurable parameter.

Thoughts?

@JakobEngel
Copy link

dynamic_reconfigure is a nice tool for setting such parameters, and actually really easy to integrate.
Only problem i see is that parameters controlled via dynamic_reconfigure cannot be changed any other way.

That is, parameters "managed" via dynamic_reconfigure will exclusively be changable by the reconfigure_gui, which is kind of anoying (e.g. cannot automatically reset these parameters to default, not even when restarting the custom node).

Only workaround i know of is using a system call from within the code, which is kind of ugly:
http://www.ros.org/wiki/hokuyo_node/Tutorials/UsingDynparamToChangeHokuyoLaserParameters#Using_dynparam_from_C.2B-.2B-_code

@mani-monaj
Copy link
Member

These are my recommendations:

  1. Skip using dynamic_reconfigure. In addition to what described by @JakobEngel, It would add a set of new dependencies/complexities to the package which will make maintenance harder in the future.
  2. I agree with the general idea in Enable Zero-Command without Hovering #34, I think if you want to write a controller, it is more convenient and natural to just toggle auto hover using cmd_vel compared to toggling a parameter and invoking a service call. This is also how SDK originally invokes that.
  3. For other parameters, I would suggest to implement @mikehamer 's idea as a service. The only caveat is reading params from ROS parameter server is expensive. As ROS services are synchronous calls, the interruption caused by update_configurations may not be negligible.
  4. If someone wants to create a GUI to configure parameters of the driver. This can be done in another ROS node/package.

@JakobEngel
Copy link

dynamic reconfigure is good for parameters that

  • are only changed interactively by the user
  • can be changed at any point in time
    so i think for the drone parameters its actually not a bad idea to use it. in particular because it already comes with a nice gui :)

and anybody should have it installed already anyway, because it is one of the default packages or something like that (everybody uses it, and everybody will immediately know how to use it).

its easy to imprelent, basically u get a callback function which is called every time a parameter is changed, in which u can then directly send the new configuration to the drone.

@mani-monaj mani-monaj merged commit cb4179f into AutonomyLab:dev-unstable Nov 23, 2012
@mani-monaj
Copy link
Member

I must thank @mikehamer for this valuable contribution to the driver. I really liked (and amazed by) your idea of using auto-generated templates to publish all navdata.

Although I manually merged your changes to dev-unstable, @github closed the pull request (without an option to re-open), so we'd better move our discussions about features to a different topic.

@mani-monaj
Copy link
Member

Some issues regarding the auto code generation:

  1. None of ROS standard message Headers are being filled up. Header.stamp & header.frame_id
  2. Isn't it better to make ardrone_autonomy::${item['struct_name']} msg; (line 66 of the template) a class member or static variable (of course name changing is inevitable)? I think the overhead of extra memory usage caused by this decision can be exempted compared to the cost of re-declaration of a complex data type.

@mikehamer
Copy link
Contributor Author

Good Point, I will change this now to a class member variable. Also thanks for picking up that about the header. I never use this data so I hadn't realized.

Also, as per your comment in @JakobEngel's discussion, I will change the way hover enable/disable works.

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.

4 participants