-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
AP_TECS: Fix Regression of TKOFF_IGAIN #28352
Conversation
The referenced commit relates to issue #27147 which surprisingly is still open. |
I just learned that the last item in the AP_GROUPINFO tupple is the default value, AP_GROUPINFO("TKOFF_IGAIN", 25, AP_TECS, _integGain_takeoff, 0.3f), Anyway the code and the default parameter must not stay this way. |
Ok, that's the NO_CODE_CHANGE version, only update parameter to 0.3f . |
@menschel can you post a link to the log which shows the problem on your aircraft? A log with the older code before this changed would be good too |
@menschel also note this should be a single commit to fix that line - assuming that's the correct fix :-) |
The referenced commit below did remove an is_zero() check which in my case made a belly landing out of an auto takeoff because the parameter TECS_TKOFF_IGAIN is 0 (default) and pitch error is not corrected. fixes: b163e13 (AP_TECS: Fixes to reset state )
6f54322
to
a0aa7f2
Compare
I can share the belly landing but honestly it was the first attempt to auto takeoff the plane and there was much to improve. I found the "suspected" TECS_TKOFF_IGAIN issue by accident. I made a follow-up flight with minor improvements where I managed to prevent ground contact with stick mixing but in auto even with stick mixing the elevator did hardly move nose-up while it moved the complete way nose-down. That's where I started looking into TECS and experimented with TECS_TKOFF_IGAIN and voila. If not zero, elevator does move nose-up the complete way and goes to neutral when 15deg pitch are reached. EDIT: EDIT2: |
Hi @menschel, The log you uploaded is a flight with ArduPlane 4.5.6. This is a stable version where the changes of #27174 are not applied yet. Also, in this log, TECS was commanding a positive pitch up, but the attitude controller didn't manage to serve it: Perhaps because your airspeed was still quite low? Could you please double-check your logs and try to explain better why this change of |
Hello, The parameter description in Missionplanner corresponds to the current text in master, so I expected the change to be present. If it's not, I have to do more test flights to iterate on the reason. All I can say for now is that dry running the whole thing on the bench, elevator moves completely nose-up when I have a non-zero value in TECS_TKOFF_IGAIN. I'm waiting for good weather because the plane is vulnerable to strong inconsistent wind. |
The code you're running currently is this, for Plane 4.5.6: https://github.com/ArduPilot/ardupilot/blob/Plane-4.5.6/libraries/AP_TECS/AP_TECS.cpp#L838 In this version, during a takeoff, when By the way, I notice that you don't use an airspeed sensor. Instead, here's a plot of your GPS speed. |
Hello, If I falsely accused your commit, I'm sorry. I do have a BIN file of the follow up test where I managed to avoid ground contact with stick mixing in auto but it's too big for upload. Is there a way to cut the BIN files to only have the relevant takeoff? I'm struggling with file sizes to upload. Changes were no motor startup delay, 15m/s² threshold and no flaps. It basically denied pulling the nose up the whole takeoff and I had to yaw off to the side to avoid the trees. I still suspect TECS. If wind calms down at 17:00CET, I'll be out flying again. |
You don't have to apologize. We need to get to the facts, and see how we can find a solution from there :) You can paste a Google Drive link or similar. It won't be live forever, but it will be available for long enough. |
Hello, I have to waive the flag here. Even after manipulating on the TECS I-Gain Values, the behavior remains. I will try flashing "Latest" Firmware, just to see if there is any change. All I can say is that it's like flying a complete different plane in FBWA and Auto. The down-slope after throwing seems to be air-frame specific but it's tame in FBWA while significant in Auto. I doubt it's the motor start delay. Anyways, when I find out what it is, I'll create new Issue or PR. Thanks for bearing with me. |
Just for info, I'm onto something. I have a negative hdem in auto takeoff and found a comment towards that behavior. https://github.com/ArduPilot/ardupilot/blob/Plane-4.5.6/libraries/AP_TECS/AP_TECS.cpp#L556 In my case it even drops the nose to negative pitch. The reason seems to be that I throw the plane with up to 20deg pitch which is higher then the 15deg in takeoff mission item. I will verify that during next flight and eventually push PR to limit hdem to positive values during takeoff. |
Before you dive too deep into TECS, I suggest you verify that your have good pitch control. Compare the ATT.Pitch and ATT.DesPitch. |
Tying some loose ends here. Additionally SCALING_SPEED was at 15 while it should have been around 10 and my ignorance got the better of me by disabling speed scaling for auto takeoff in FLIGHT_OPTIONS because I don't have an airspeed sensor. After getting rid of this concatenation of unfavorable conditions and setting TECS_TKOFF_IGAIN=0.4, the Drift does auto takeoff without even wagging the tail regardless of what pitch I throw it. |
Nice to hear! |
I will check with 0.02 because I'm still using AP4.5.7. The main issue has been the reflex on the elevator. ZOHD designed the plane to be very nose heavy for beginners and counter with reflex on the elevator. From my analysis, a high TECS_TKOFF_IGAIN helps fixing the TRIM issues when the plane's nose dips down initially after throw. I currently takeoff at 12° pitch because I fly a configuration with a DIY 1400mAh Li-Ion Pack, 249g weight, and performance limitation to 3A (5A Burst for 10 seconds). Flight time almost 40 minutes with battery failsafe at 6,2V and 1000mAh. |
I tested with current 4.7.0-dev because I wanted the mavlink connection via ELRS, I did not notice any difference in the climb ability or the initial pitch correction but the plane did seem to not reach the target altitude for TKOFF, it kept flying at 48m, 2m below at almost horizontal pitch. I'm not sure about the course either, seemed to be shifted by crosswind and was not corrected. I returned in FBWA. There was also some other issue on the GCS messages, I get the initial (board,version,rc-out config) tuple repeatedly, so I assume I should not fly latest software. And I was seeing a 2nd vehicle in APM Planner. This did not happen in 4.5.7 . To sum it up, I still vote for a TECS_TKOFF_IGAIN default of 0.3. EDIT: I also do not seem to have a bin log for that flight on the sd card. |
Okay. My biggest concern with But alas I'd need a .bin log to comment on your altitude issue. Thanks for reporting back! |
I'll check again with https://github.com/ArduPilot/ardupilot/tree/ArduPilot-4.6 Not sure what the issue was with 4.7.0-dev. |
No issues with AP 4.6.0-Beta1 yet. I made one takeoff into ~15-20km/h Headwind and did not see anything unusual with I'll do another flight when there is less wind. |
That's good to hear. Thanks a lot for reporting back, I really appreciate it! |
Bad news, I have a tkoff with 4.6.0-beta1 and it leveld out below target altitude again. I went on to stash things into an existing repo, so I don't have to ask Seve to delete it afterwards. |
I got the wrong switch when trying to switch to FBWA and moved flaps 50% instead. The stick action after that was intended because I wanted to turn. Then I realized I was still in auto and flipped the correct switch to FBWA. The 100% THR is not normal. It should have reduced to ~75% because 12deg (TKOFF angle) / 18deg (TECS pitch max) results in that value in 4.5.7. The flaperons are quite effective, I just need some feed-forward to pitch control. That flight also had ~25km/h wind which is more then the minimum speed for the plane. It's a bumpy ride for a sub 250g 880mm wingspan plane. |
Where are you basing this off of? I don't recall such a feature. Especially in TAKEOFF mode with See the documentation for the refactor ArduPilot/ardupilot_wiki#6149 |
I run throttle controlled without airspeed sensor and TECS is setup according to guide. Means TECS_CLIMB_MAX corresponds to TECS_PITCH_MAX at 100% THR. Less Pitch scales throttle down in 4.5.7. |
See my updated comment above. |
Was that an intended change? Because in 4.5.7 it drops down when the time has exceeded and does not care about altitude. As I wrote, I have constraints on the discharge curve and the maximum speed of the airframe which ZOHD names as 50km/h although it can do ~60km/h horizontally. |
The change in behaviour is intentional because the behaviour in 4.5 was buggy.
What you quoted here is true when you set
That's OK, it shouldn't prevent you from completing a takeoff and I'm looking onto writing a patch. |
I have a suspicion. TKOFF_ALT and TKOFF_LVL_ALT copy paste error somewhere. |
OK, so what I take as expected behavior is the bug mentioned here. |
I'm afraid for now they won't really help you. These parameters won't have a beneficial effect in your configuration. What you can do, as a mitigation, is to set |
I'll check what I can fix by using TECS_TKOFF_IGAIN=0,4. But honestly, why is Airspeed Sensor Presence evaluated here? If TECS is setup correctly the resulting throttle value would yield cruise speed regardless what takeoff angle is. It works in 4.5.7 even though labeled a bug. |
That was a pre-existing design decision, not done by me, in order to make 100% sure that you won't stall your plane during takeoff, especially in absence of an airspeed sensor. We could amend it, but that's for another discussion. By the way, I have a proposed fix for your climb failure here. |
Thanks, I'll cherry-pick onto 4.6.0 branch and build. |
I have just updated the description there, in case you want to read more about it. |
Thanks, just for the record, it's this plane, second revision. Another interesting thing what I noticed in my FPV footage, the level-off already starts at 8m instead of 3m previously. I assume that's the initial cause for the altitude undershoot. I'll test tomorrow if I find a free slot and the wind is <20km/h. |
@menschel I'm sorry to report that that PR was flawed. Feel free to have another look at the code, but I think I'm double dipping on the pitch trim parameter. I guess I'll have to write some sort of other controller, but for now I have no other solution for you other than setting |
Ok, then I'll test TKOFF_THR_MAX=75% and TECS_TKOFF_IGAIN=0,4 instead. The IGAIN should make it reach target altitude. |
It will probably make no difference, because the TECS pitch is not taken into account when you don't have an airspeed sensor. You get the fixed takeoff pitch, as set in the mission item. |
for reference ap460beta1_tkoff_issue.mp4 EDIT: I also notice another thing here. The plane does not keep the original course but turns roughly 30degrees clockwise after the throw. |
The referenced commit below did remove an is_zero() check which in my case made a belly landing out of an auto takeoff because the parameter TECS_TKOFF_IGAIN is 0 (default) and pitch error is not corrected.
fixes: b163e13 (AP_TECS: Fixes to reset state )