Hybrid ready for review by Julien, SandroT before being pushed to master

[Randy Mackay]

Julien, SandroT,

     I’ve reworked Hybrid mode and I was wondering if you could have a look at it and give it the “ok” (or “no way!”) before I push to master?

                 https://github.com/rmackay9/rmackay9-ardupilot/tree/hybrid3

 

     I’ve tested it in the simulator and on an Iris/Pixhawk and it seems to work.  It should be nearly identical to your original code but I changed the structure quite a bit.  The changes are:

·         Split the roll/pitch modes into 6 states (previously there were 3).  I personally think this makes it clearer what state the roll and pitch modes are in.

PILOT_OVERRIDE

BRAKE

BRAKE_READY_TO_LOITER

BRAKE_TO_LOITER

LOITER

LOITER_TO_PILOT_OVERRIDE

One bit that is confusing is that BRAKE_TO_LOITER and LOITER are combined roll-pitch modes. I.e roll and pitch must both be in the mode at the same time.  This means that there end up being 3 big switch statements in control_hybrid.pde.  One for the roll-mode, one for the pitch-mode and one for the combined roll+pitch when we’re dealing with BRAKE_TO_LOITER and LOITER.

·         Moved some code out of hybrid_run() and into separate functions at the bottom (i.e. “hybrid_mix_controls()” and “hybrid_update_wind_comp_estimate”).

·         Setup #defines for 100hz and 400hz update rates

 

     There were some parts which didn’t fit in with the ardupilot coding standards.  These standards are undocumented but they will appear on the wiki soon.  In order to contribute that wiki page here are the issues that came up:

·         Don’t cast to (long) or (int) but instead cast to (int32_t) or (int16_t) because “long” and “int” are different sizes on different platforms.  The only time that (long) or (int) should be used is within printf statements because in those cases the print_f is implemented by that platform so it needs the variable to be of the type that matches the %ld or %d that appears in the printf’s format string

·         Some #defines and parameters were in the WPNav library but they were used only in the control_hybrid.pde file.  I’ve moved these to the main vehicle’s parameter list.  I do think we should move hybrid and Loiter into a new library eventually though.

·         Be sure to rebase on master

·         Commenting is great but it’s best not to include large blocks of commented out code because it makes the code harder to read

·         Use 4 spaces instead of tab

 

-Randy

[Julien Dubois]

Randy,

as you know, I've published my reviewed branch on your repo here
https://github.com/rmackay9/rmackay9-ardupilot/tree/hybrid3-JD-Review

I've tested this code and, from my in-flight feeling, it works well but not perfectly... I've seen start and go effect after braking, very strong brake angles on pitch and some throttle peaks.

After checking logs, the desired_roll/pitch are just fine, there is maybe a problem with my copter's tuning (I just used the params I had for pre-onion code that work perfectly). Or there is maybe a problem with stab_controller... anyways hybrid code send the expected roll/pitch commands to stab controller.

Wind_comp seems to work fine as well.

Another problem I noticed is sometimes a peak on throttle_out due to an unexpected DesAlt increase (Altitude, ThrIn and RC_3 being constant).

Logs and screenshots attached.
Anybody knows if Onion need different tuning on roll/pitch/yaw than pre-Onion? I felt Yaw a bit slow as well.

Any feedback welcome.
Julien

PS: Sandro, please test it and give your feeling :)

[Peter Plischka]

Hi Julien,

I am yesterday flown this version shortly on my Hexa. 

The transitions from Loitern are beautiful gentle.
But the throttle peaks are strong. That looks dangerous.

Peter

[Julien Dubois]

Thanks Peter for feedback.

Could you please attach your log here to check if hybrid command are the expected ones and compare your throttle peaks wy mine.

BTW, I realized CLI was disabled for APM so we cannot get flight logs even if logging is enabled.
So, in case of APM, you just have to enable CLI in APM_Config.h (and disable unused features to save dataflas < 242kB), load the code in the controller and get the flight log.

Throttle_controller used in hybrid should be the same as Alt_Hold, if you could make some test in alt_hold as well to check if the problem occurs.

Julien

[Peter Plischka]

There is only a short log.
I have a PIXHAWK on the hexa and can load the DataFlash logs without CLI.
In Alt Hold i have  nothing noticed, the throttle Peaks are only in Hybrid Mode.

Peter

[Peter Plischka]

Hi Julien, 

another flight.
Alt Hold is absolutely fine.

The peaks occur more often with EFF = ON.
In the log of the first third I flew with EKF = Off and had no peaks.
With EKF = ON peaks  were immediately there.

Peter

[Julien Dubois]

Ok thank you Peter,

I've compared Hybrid/Alt_hold and loiter control_mode codes and the only difference I see is the data type used in loiter and hybrid (float) vs int16_t for Alt_hold.
An init was not properly done in hybrid neither but I doubt this could explain theses peaks.

So, I've changed the code and will try hybrid with the int16_t data type. then compare with alt_hold and loiter that I let with the different data types.

Hybrid requires more CPU time than the other flight modes, maybe in some cases it is not properly run because of scheduler limits... and then we should increase the elapsed time in the scheduler?

Well, I let you know my feedback asap and publish the commit if better.

Julien

[Randy Mackay]

Julien,

     Sorry, I haven’t completed reviewing the fixes you’ve made to the post-onion-hybrid in my branch.  Hopefully tomorrow.

 

Peter,

     I don’t think testing Hybrid with EKF=ON is a good idea.  There could be a lot of differences between DCM and EKF and it will complicate the testing.

 

     Thanks for all the testing and reviewing.

 

-Randy

--
You received this message because you are subscribed to the Google Groups "drones-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to drones-discus...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Julien Dubois]

Ok Randy, no problem,
I've made another commit to fix some things I've seen in Peter's log (very good test and log BTW Peter!!)

There was an error in the velocity axis used to update brake_timeout_pitch (vel_right instead of vel_fw)

The wind_comp was not enough filtered for the Pixhawk (400Hz), so I added a specific time constant (TC_WIND_COMP) to have the expected filter with 400Hz controllers.

About throttle peaks, after some tests and from logs, they happen when hybrid switches to loiter.
There is always a difference between Alt and DesiredAlt (DAlt), but, when loiter engages, it initializes DAlt = Alt and the copter tries immediatelly to reach that new setpoint with a peak. So the solution would be to init_loiter_target() just as it was in pre-onion code : only x/y and not z. and to be able to pass parameters like that
wp_nav.init_loiter_target(inertial_nav.get_position(), Vector3f(0,0,0));

Well, from this new code structure, it seems not possible with current functions so I've used set_loiter_target that initializes position passed as parameter and velocity to 0 (as expected).
BTW, I think there was something wrong with set_loiter_target function, the "Vector3f& position" parameter was not used at all...
I moved the "reset_I" flag from init_loiter_target to set_loiter_target.

This new code has not been tested for the moment, I hope it will fix the throttle peaks.

Randy, for sure there is something easier to do but before we could give a try to check if this fixes the bug completely.

Thanks for your feedback.
Julien

[Peter Plischka]

Thank you Randy and Julien,

I can test the new version this afternoon.

regards Peter

[Peter Plischka]

Hi Julien.

absolutely perfect. No peak in height.

That was really fun. I meant to fly a DJI. I think hybrid is a great addition for us. 

Now I have to deal with the hybrid parameters, but the default Settings are very well.


Peter

[Julien Dubois]

Hi Peter,

I'm glad to hear it! Thanks a lot for testing.
I'll check in your log if everything is fine now but I think we are on the right track :)

About params, we've made many tests to find out the best values and, really, the default ones have given very good compromise between smooth and braking efficiency.
In this onioned version, there are the basic params only, others are now fixed in the code because we've seen no interest keeping them (and anyways : the simplest, the best).

Well, now it's maybe time start intensive testing.
Marco, your feedback would be welcome!

Julien

[Peter Plischka]

No Problem Julien.
I hope it comes quickly in the master.

I had changed the parameters and did some quick flights. But you're right. The default parameter fit very well for me.

Peter

[Julien Dubois]

Randy,

I did a flight and everything is OK

From Peter's log, all is fine as well, the identified problems seem to be fixed by last commit.

By now, the code has been tested on APM and Peter's Pixhawk so both 100/400Hz controllers have been checked.

Peter, nice you came to the same result on params!

Anyways, more tests/feedback welcome.

Julien

[Andrew Chapman]

Did we get any closer to resolving the name?

I think the proliferation of flight modes is confusing and probably hurting us. I'd encourage the new mode to be called something like 'LOITER2', 'LOITER_HYBRID' or 'LOITER_SMOOTH' so that it is self-evidently the same kind of thing as LOITER. And if down the line we end up replacing the existing LOITER with this one it won't be too confusing.

HYBRID was a good name while it was in dev, but it only makes sense if you know what things it is a hybrid of.

AC.


On Sunday, April 13, 2014 12:49:59 AM UTC-7, Randy Mackay wrote:

     I’ve reworked Hybrid mode...

[Josh Welsh]

Agreed.  For as descriptive as HYBRID is, we may as well call it “GENERIC” or “FLIGHT_MODE.”  HYBRID in this case will create a lot of support questions around its capabilities and intent, and I think it best to do clear that up before the rush starts.  Case in point, look at Marco’s post on DIYD.. the conversation went quickly from “Wow that’s cool” to “Let’s name it this” and “let’s name it that..”

 

I personally don’t have a strong opinion on what the name should be, other than it shouldn’t be called HYBRID and should somewhat describe the actual flight mode.

 

From: drones-...@googlegroups.com [mailto:drones-...@googlegroups.com] On Behalf Of Andrew Chapman
Sent: Tuesday, April 22, 2014 2:39 PM
To: drones-...@googlegroups.com
Subject: [drones-discuss] Re: Hybrid ready for review by Julien, SandroT before being pushed to master

 

Did we get any closer to resolving the name?

--

[Jason Short]

If everyone's into doing renames, from the 3DR side, we'd like to revisit all the flight mode names so they are a more of a cohesive family.

It would make explaining features to users much easier if the names were systematic rather than a dog's breakfast.

Jason

[Josh Welsh]

Jason, 

I propose we keep the initiative of renaming all current flight modes separate from the one of launching a new flight mode with an agreeable name. That said, I'm personally very curious on hearing your thoughts on new names, perhaps in a new thread?  I certainly have some ideas as well but also think some are very appropriate for what they do (auto, RTL, etc)

[Randy Mackay]

Julien,

 

     Ok, great news.  I’ve merged it all into master.

 

     I did some minor cleanup of comments and also I changed how the reset_I stuff was working.  I moved the flag down to the position controller level and also made it automatically reset by to the default (which is “true”).  I tested it and I don’t think I introduced any bugs as part of the merge but to err is human so if you see anything….

 

      By the way, we’ve run out of Flash space on the APM1/2.  I’ve at least temporarily disabled the optical flow sensor to make room for Hybrid.

 

-Randy

 

From: drones-...@googlegroups.com [mailto:drones-...@googlegroups.com] On Behalf Of Julien Dubois
Sent: April 23, 2014 6:08 AM
To: drones-...@googlegroups.com
Subject: [drones-discuss] Re: Hybrid ready for review by Julien, SandroT before being pushed to master

 

Randy,

--

[Marco Robustini]

I agree with Andrew.
At the time, I proposed to create a flag for the current Loiter have the option to make it "flyable", APM is increasing dramatically the number of flight modes and this confuses some users.
"Hybrid" mean just wanted a mixed mode between Loiter and Alt-Hold, but the name really does not really describe what the flight mode offers.
But also "Loiter", which translated into my language means "shaking move without a specific destination" (maybe it makes sense for a plane, not for a copter) is not to be the top ... ;-)
So i suggest:

Loiter -> Nav Hold (because using the NAV controller for repositioning)
Hybrid -> Position Hold

My two cents...

Marco

[Julien Dubois]

Ok Randy,

thanks a lot for your job!
I'll check it asap.

About the name, Sandro and I liked the name "hybrid" but, indeed, it's not specific enough.
The idea was to say: that's an alt_hold/loiter smooth combo mix.

Well, when you see DJI controllers, their flight names are very basic and clear, only 3 : GPS Atti, Atti, Manual.
So, we've a working/explicit example, why not to learn from it?

This way, "GPS AltHold" would be a kind of equivalence..

Julien

[Randy Mackay]

I would vote for:

   Loiter -> Position Hold

  Hybrid -> Position Hold2

 

I think eventually we will merge them together or replace regular Loiter with Hybrid.

 

-Randy

 

From: drones-...@googlegroups.com [mailto:drones-...@googlegroups.com] On Behalf Of Marco Robustini
Sent: April 23, 2014 4:01 PM
To: drones-...@googlegroups.com
Subject: [drones-discuss] Re: Hybrid ready for review by Julien, SandroT before being pushed to master

 

I agree with Andrew.

--

[Jesus Alvarez]

I think names should be as self explanatory as posible. So adding a "2" at the end doesn't seem explanatory to me :)

For me GPS position hold should be an addition to any other manual/semimanual flight mode in the same way as "simple" is.
Understanding this option as a gps position hold when user has sticks centered/released.
It would be a great addition to stabilize, to althold (the becoming hybrid), to drift etc etc. The pilot will always be sure that once he release the sticks the copter will stick to that position (if he has enabled the option).

[David Pawlak]

Hybrid is just a Loiter that works like it should!

Loiter is Position Hold!

[Scott Simpson]

Awesome job on this new mode!  I just tested it (from master) on my hex/pixhawk in the strong wind and it flew great. 

Just for test purposes, I decided to land on soft grass while still in hybrid.  It immediately topped over with the motors spinning.  I personally never land in anything other than stabilize, but thought I'd try it since this is what people familiar with DJI will do.  If anyone wants any information or thinks this is something that should be pursued, I'll be happy to risk my hex (or at least a few blades) to test landing in hybrid.  Otherwise, I'll try to test other things.

Thanks again,

Scott

[Julien Dubois]

Scott,

if you had an issue, please attach the log.
I'm surprised and don't understand why it tipped over, maybe a GPS glitch if landing in loiter.
I take off and land in hybrid for months without having a single problem (baro disturbances excluded, not related to hybrid code anyways).
Basically, when you land with a copter you should have no more x/y velocity so there are 2 cases :
1- you stopped moving sticks and just lower throttle => this is a loiter land. In this case, a little GPS glitch could move target pos + a feet blocked in the grass =>done
2 - fingers moving slowly sticks => this is an Alt_hold land x/y being controlled manually, that's just like a stabilize land.

Maybe motor sync loss?

Please, give more details of what happened exactelly and the log.

Julien

[Julien Dubois]

BTW, which "master" are you running?

@Randy : I've reviewed your hybrid5 with a hybrid5-JD-Review on your repo.

[Scott Simpson]

Thanks, you've helped already and I will keep testing landing and report back with logs next time.  I didn't have the motors in the logs I captured tonight so I've enabled them and am trying to reproduce the problem.  

You mentioned that you can take off in hybrid.  My motors won't spin up at all in hybrid or alt-hold.  I have to start flying in stabilize always.  I'll look for a setting for this.  

I'm building hexa from the github diydrones master dev that was pushed up this morning.

Thanks,

Scott

--
You received this message because you are subscribed to a topic in the Google Groups "drones-discuss" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/drones-discuss/WEjetP2Iq58/unsubscribe.
To unsubscribe from this group and all its topics, send an email to drones-discus...@googlegroups.com.

[Julien Dubois]

Scott,

even if motors log was disabled, just give the log you have, there may be some piece of answer

Thanks

Julien

[Scott Simpson]

I attached the log of the flight that spun into the ground after landing.  I don't want to take up too much of your time on this log though.  Now that I know I should be able to land (and take off?) in the the new mode, I'll focus testing on that but with motor logging data next time.  

I'd like to get setup to take off in the new mode without going through stabilize first.  Do you know what is more helpful?  1) Wait, 2) Fly your git repo, or 3) Modify my source?  (I'm a software developer)

Thanks,

Scott

[Julien Dubois]

Scott,

I supose the issue happened before you switched to stabilize.
Roll/pitch angles seem not critic from log before you switch to satbilize... The RCIN log would give the answer on what happened.

Your hybrid params are fine and the crash did not happen in a loitering state so in a manual or braking state.
From des_roll/pitch shapes, it could not be the braking state (slope curve too high)
So, with these elements, I just see a bad manual command or motor sync loss.

Sorry, I really have no clue, maybe a more watchful eye could be more helpfull.

Julien

PS : you should be able to take off and land in every mode.. if you don't, check your params (compare to default ones in mission planner)
About which code to flight, I don't know if lastest Randy's commit has been validated by anybody.
I'm just sure of this one I tested with no problem:
https://github.com/rmackay9/rmackay9-ardupilot/tree/hybrid3-JD-Review
I've disabled some features in APM_Config.h because of APM memory limits. you can enable them if you run another controller

[Marco Robustini]

I'm sorry for the delay, tomorrow my tests with the 3.2 trunk and of course, with Hybrid and EKF.
I moved my attention lately to APM Plane for some reason that i will not explain, now i'm back for final tests with the copter version... ;-)

Cheers, Marco

[Scott Simpson]

Thanks Julien,

If I report any more problems, I'll make sure to have a better synopsis and a complete log.  

Best Regards,

Scott

[Julien Dubois]

Randy,

I've just flown hybrid of your master repo, the flight feeling was great even with strong wind.
Unfortunatelly there's no hybrid log to check all the details but the last changes should not have introduced any bugs in the code, I would have seen them in flight.
Land/take_off, arming/disarm, power on/off... no issue.

I just wait for Marco's test results but I think it's OK for being tested/run by everybody.

Julien

[Peter Plischka]

Hi there,

I flew the last few days with Hybrid. I have still a lot of fun!.

But I'm still irritated by the Throttle peak when switching from ALTHOLD to HYBRID and vice versa.
He is not strong, maybe 5 cm.
Most it bothers after landing when I switch from HYBRID to ALTHOLD and then to STAB while the Motors are running.

Then the copter bounces briefly in the air. 

Peter

[Marco Robustini]

Tested today, same "throttle peak" described by Peter, apart from this thing Hybrid work fine!

Marco

[Marco Robustini]

Even in my first video with VR Brain I pointed this thing with Hybrid, do you remember Julien?
See my video...

Marco

[Julien Dubois]

Marco, peaks described by Peter happen on mode switching.

In his previous post, that was during hybrid mode but that seems to be fixed... I hope you're talkinf of mode switching peak and not peak during hybrid.

Peak come from modes init. have you noticed them when switching from alt_hold to loiter and vice versa?

Randy, In Peter's log, both Hybrid and Alt_Hold are called Alt_hold... In my log, Hybrid is called Hybrid, do you know why?

--

[Peter Plischka]

Julien,

still Clarification: 

I had used the current master.

In the beginning there was the peak during the flight. 

That is completely gone. 

Now there is a peak at the mode switching. 

Peter

[Julien Dubois]

Actually, Hybrid, loiter, Alt_Hold are calling the function pos_control.set_target_to_stopping_point_z(); that set (more or less) _pos_target = cur_pos_z

If you check your log whith Alt, DAlt and throttle_Out, especially lines 108000 to 113000, you'll see there is a difference between DAlt and Alt and your target position is increasing with your different mode switching because your copter tended to be at a 7-8cm higher Altitude than Desired.

Well, imo there is no interest calling the pos_control.set_target_to_stopping_point_z() function in a control_mode if our previous mode was already Alt_controlled.
So, there are 2 solutions :
- in each control_mode, test previous control_mode and call the function in some case
- in AC_PosControl::get_stopping_point_z(Vector3f& stopping_point) function, just set pos_target = current alt in some cases (target_pos.z=0 or alt_error>20cm... maybe 15cm)

I'll make a fix for it to be tested of course.
Maybe Randy will have a better solution but we can give a try to check if this solve the issue.

Julien

[Julien Dubois]

Marco, Peter, if you want to try this tomorrow
https://github.com/rmackay9/rmackay9-ardupilot/tree/hybrid6

I'll give a try as well if weather allows it.

Julien

[Randy Mackay]

Julien,

 

     I think this is a pretty good idea.  The Guided mode twitch was basically the same issue but in the horizontal plane.  Again it was using the ‘stopping-point’ function instead of the existing position target.

 

     The only thing is we probably need to be careful that the position controller is active, so we may need an active() method which returns true if the position controller has been controlling the horizontal or vertical position within the past 0.2 seconds (or whatever timeframe works).

 

     We should probably go through and check all the other places we use get-stopping-point and see if we can replace them too.

 

-Randy

--

You received this message because you are subscribed to the Google Groups "drones-discuss" group.

To unsubscribe from this group and stop receiving emails from it, send an email to drones-discus...@googlegroups.com.

[Peter Plischka]

Julien,

I have taken hybrid6.

during  mode switching in flight, there is no Paek. - This is now fine

Often the landing detector does not detect the landing. 
If I switch immediately after the landing from HYBRID to ALTHOLD the copter  jumps up a few cm. 

Peter

[Julien Dubois]

Peter,

thanks a lot for testing.
Code works as expected, when you switched to alt_hold once landed, there was an alt_error > 20cm, that's why pos_target.z has been set to curr_pos.z.

So, I think Randy's solution is the right one, it would be the most safe and general condition to update or not pos_target.

Julien