-
Notifications
You must be signed in to change notification settings - Fork 226
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
Changing parameters will now affect the drone, rather than doing nothing! #26
Conversation
I've fixed the bug causing parameter changes to have no effect on the drone. Now we do a proper setup of the drone tool (rather than using the factory default), which includes setting up application and user profiles on the drone to hold our settings. We then actually send the parameters to the drone, rather than doing nothing. The parameters system has been extended to allow all user-modifyable parameters to be changed (see AR.Drone SDK manual).
@@ -0,0 +1,22 @@ | |||
<launch> | |||
<node name="ardrone_driver" pkg="ardrone_autonomy" type="ardrone_driver" output="screen" clear_params="true"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should add the clear_params="true"
line to your launch file, otherwise removing parameters will have no effect as the values still exist in the ros parameter server.
Thank you very much @mikehamer for the interesting patch. I will test the changes today. |
I merged your proposed changes into dev-unstable branch. I also modified your code a little bit. My initial tests using the new code were all good. I would really appreciate it if you test the I also have a question. Is the current
I also think that that the sample launch file should include values close to default values. I added a warning to the beginning of the file because I think novice users may get confused. What is your opinion? |
I agree about the default launch file, perhaps (see end of this msg), we just include the suggested parameter changes, and set them to the default values? Important is the clear_params="true" though, it took me a while to realise why the drone parameters weren't getting reset to default if removed from the launch file. The README is correct, if the parameters are not set using GENERAL CONTROL NETWORK PIC VIDEO LEDS DETECTION SYSLOG USERBOX GPS Many of these parameters should probably not be changed... however they are exposed for advanced users to change if they want. Of course, if they are not set, they will remain default. Perhaps a good idea would be to provide a list of "suggested parameters to change", which would include Suggested Parameter Changes video_codec enemy_colors Of course, the SDK manual details all of these parameters and how they should be changed. I'll test the changes and get back to you on Monday with results. Cheers, |
Thank you very much for helping me with this. One more thing needs to be considered. Not all params are writable, things like firmware version are read only according to SDK2 manual. |
This reverts commit 3ec0db0.
I put a further check on the parameter parsing code, which checks the parameter definition to see if it is writeable before trying to find a ros parameter
@mani-monaj: No problems – I'm using the driver for my master's project and needed to be able to change drone parameters; these changes are as much beneficial to me, as they are to the other users! In #27 you will find a pull request that implements a fix to your comment – now only the writable subset of parameters will be loaded from ros parameters. |
Now only the following parameters will be checked: ardrone_name |
I reset the very aggressive flight parameters to the defaults (eg relatively slow and controllable)
I forgot to remove the debug output from the launch file last time
Regarding testing the dev-unstable branch, I flew for an hour today (multiple flights) using my modifications (above) to the dev-unstable branch and had no issues. |
Optimally this would be calculated before being published to Navdata, but for backwards compatibility lets document the format for now, and discuss an additional unpacked time
Now only loads the writeable subset of drone parameters
Thank you very much. I merged #27 into I think the only difference is that, the "multi-configuration" is implemented different than what SDK2 recommends. The driver will reset all the configurations (SESSION/APLI/COMMON) to default at startup, then reads |
Sorry for the commit, I wanted to switch back to the main branch for a while
This reverts commit f46a7ad.
Updating the README with all the available variables to be included in a launch file would be very helpful considering the density of the SDK reference. |
Thanks for the feedback @parcon. I'll try and update it this week (at least on my fork). I'll also be adding some more functionality on the navdata side of things. For both of these changes I will submit a pull request to this main repository. |
@parcon @mikehamer Do you mean listing only the name of all params? I was thinking of including the default values (or maybe a short description) as well, but I am afraid that it will take lots of space in the README file. @mikehamer Is the claim in my previous comment correct to be included in the README? |
@mani-monaj The list I posted in my previous comment is the list of all parameters which are checked. The list is all the drone's Read/Write (not readonly as you stated) parameters. |
I feel either the list or a list with explanation would be helpful for those interested in getting the drone to work under ROS. The more the drivers don't seem like a blackbox, the more contributions to the code I would expect. |
Thanks.
I will include the small list in the REAMDE. I may create a wiki page for the long list with explanation and default values. In the meantime people can use the SDK2 manual to find the explanations and default values. |
I've just generated the following, perhaps it will be of help for the wiki page. These are all the parameters which are checked, along with their default values, both as defined through macros if applicable, and numerically, as should be entered in a launch file. altitude (Default = 0) altitude_max (Default = 3000) altitude_min (Default = 50) ardrone_name (Default = My ARDrone) autonomous_flight (Default = 0) bitrate (Default = 1000) bitrate_ctrl_mode (Default = 0) bitrate_storage (Default = 4000) codec_fps (Default = 0) com_watchdog (Default = 2) control_iphone_tilt (Default = (20000.0f * (MDEG_TO_RAD)) = 0.349066) control_level (Default = 0) control_vz_max (Default = 700) control_yaw (Default = (100000.0f * (MDEG_TO_RAD)) = 1.745329) detect_type (Default = CAD_TYPE_NONE = 3) detections_select_h (Default = 0) detections_select_v (Default = 0) detections_select_v_hsync (Default = 0) enemy_colors (Default = (ARDRONE_DETECTION_COLOR_ORANGE_GREEN) = 1) enemy_without_shell (Default = 0) euler_angle_max (Default = (12000.0f * (MDEG_TO_RAD)) = 0.209440) flight_anim (Default = 0,0) flight_without_shell (Default = 0) flying_mode (Default = 0) groundstripe_colors (Default = (ARDRONE_DETECTION_COLOR_ARRACE_FINISH_LINE) = 16) hovering_range (Default = 1000) indoor_control_vz_max (Default = 700) indoor_control_yaw (Default = (100000.0f * (MDEG_TO_RAD)) = 1.745329) indoor_euler_angle_max (Default = (12000.0f * (MDEG_TO_RAD)) = 0.209440) latitude (Default = 500) leds_anim (Default = 0,0,0) longitude (Default = 500) manual_trim (Default = 0) max_bitrate (Default = 1000) max_size (Default = 102400) navdata_demo (Default = 0) navdata_options (Default = ( ( 1 << (NAVDATA_DEMO_TAG) )|( 1 << (NAVDATA_VISION_DETECT_TAG) ) ) = 65537) nb_files (Default = 5) outdoor (Default = 0) outdoor_control_vz_max (Default = (1000.0f) = 1000) outdoor_control_yaw (Default = (200000.0f * (MDEG_TO_RAD)) = 3.490659) outdoor_euler_angle_max (Default = (20000.0f * (MDEG_TO_RAD)) = 0.349066) output (Default = (UART_PRINT|WIFI_PRINT|FLASH_PRINT) = 7) owner_mac (Default = 00:00:00:00:00:00) ssid_multi_player (Default = ardronenetwork) ssid_single_player (Default = ardronenetwork) travelling_enable (Default = 0) travelling_mode (Default = 0,10,1500,0,1000) ultrasound_freq (Default = ADC_CMD_SELECT_ULTRASOUND_25Hz = 8) ultrasound_watchdog (Default = 3) userbox_cmd (Default = 0) video_channel (Default = 0) video_codec (Default = (UVLC_CODEC) = 32) video_enable (Default = 1) video_file_index (Default = 1) video_live_socket (Default = 0) video_on_usb (Default = 1) video_slices (Default = 0) vision_enable (Default = 1) wifi_mode (Default = WIFI_MODE_INFRA = 0) wifi_rate (Default = 0) |
We now have the ability to access any of the navdata structs which are sent back by the drone. Still to do is to enable/disable the drone sending based on whether we're interested in receiving the message.
Still to do is add a flag to disable the default navdata and imu messages
As per parcon's suggestion, I'm adding the ability to disable the hover mode to allow for more predictable and thus modellable drone dynamics. This should also be useful for my purposes.
This flag allows us to turn off the legacy navdata/imu/mag topics, favouring usage of the actual packets
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!
@mani-monaj The changes above this comment do 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. 2. Turn off combined navdata messages 3. Disable hover mode 4. Always send hover commands (even unchanged ones) 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. |
@mikehamer thank you very much for your contributions. Is it possible for you to move the new commits to a new pull request? We will discuss them in the new place. My roadmap was to merge the current |
I forgot to mention, please create the new pull request to |
New pull request submitted in #31, lets discuss there. |
Fix pressure sensor publication
I fixed the bug I reported a few days ago, whereby changes to parameters (from CAT_USER, CAT_APPLI, CAT_COMMON) would not affect the drone at all.
Previously, the drone was setup using a call to ardrone_tool_main(), this is no longer recommended by Parrot. I wrote a custom initialization routine. Now the driver correctly initializes its application and profile config files on the drone. Then, once initialized, the driver pushes the new configuration values to the drone.
I have tested this in flight. This also solves the problem with onboard tag detection not working (because now you can turn it on and change what it detects!), and allows control parameters of the drone to be adjusted (eg, euler_angle_max, control_vz_max, etc). This allows more aggressive flight with the drone.
I have also extended the parameter system to allow any user-editable parameter to be changed, rather than the limited set available on the previous driver. For possible keys, see the AR.Drone SDK manual, or the file
ARDroneLib/Soft/Common/config_keys.h
.