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

List of default params that need an update #12361

Closed
RomanBapst opened this issue Jun 28, 2019 · 11 comments · Fixed by #12543
Closed

List of default params that need an update #12361

RomanBapst opened this issue Jun 28, 2019 · 11 comments · Fixed by #12543

Comments

@RomanBapst
Copy link
Contributor

RomanBapst commented Jun 28, 2019

This is to track parameters which have a default that does not work well on practice or leads to inferior performance on pretty much every airframe.

Parameter name default suggested
MPC_Z_VEL_I 0.02 0.1
MPC_ACC_DOWN_MAX 10.0 3.0
MPC_ACC_UP_MAX 10.0 4.0
SYS_FORCE_F7DC 0 2
MPC_POS_MODE 1 3
MPC_ACC_HOR 5 3
MPC_ACC_HOR_MAX 10 5
FW_RR_I 0.01 0.1
FW_PR_I 0.01 0.1
FW_YR_I 0.01 0.1
MPC_Z_MAN_EXPO 0 0.6
MPC_XY_MAN_EXPO 0 0.6
MPC_YAW_EXPO 0 0.6
MPC_MAN_Y_MAX 200 150
@LorenzMeier
Copy link
Member

I think we need this in v1.9.2 already as we otherwise keep this issue too long.

@DanielePettenuzzo
Copy link
Contributor

SYS_FORCE_F7DC worries me a bit because the default is set to disable dcache. With dcache disabled we are very close to 100% cpu on fmu-v5 if we have a companion link, run uavcan or run the avoidance interface (many users could be hit by this). I just think that currently the risk of crashing due to a maxed out cpu is higher than actually hitting the dcache errata.

@dagar
Copy link
Member

dagar commented Jul 10, 2019

SYS_FORCE_F7DC worries me a bit because the default is set to disable dcache. With dcache disabled we are very close to 100% cpu on fmu-v5 if we have a companion link, run uavcan or run the avoidance interface (many users could be hit by this). I just think that currently the risk of crashing due to a maxed out cpu is higher than actually hitting the dcache errata.

@DanielePettenuzzo I'm pushing for a proper fix. #12435

@dagar
Copy link
Member

dagar commented Jul 11, 2019

@RomanBapst more updates coming or should we start making these changes (for v1.9.3 at this point)?

@bresch
Copy link
Member

bresch commented Jul 12, 2019

We're using MC_POS_MODE 3 (jerk-limited smoothing) since at least January on all our vehicles and I think it's the right time to change it to the upstream default. It will also help to receive more feedback from the user and the test team to improve it further.

@fabianschilling
Copy link

The default EKF2_EV_DELAY of 175 ms seems rather high for flight with motion capture systems. A value around 50-100 ms could be a better default.

@bresch bresch added this to Low priority in Devcall via automation Jul 15, 2019
@julianoes
Copy link
Contributor

julianoes commented Jul 15, 2019

@LorenzMeier

I think we need this in v1.9.2 already as we otherwise keep this issue too long.

No, v1.9.2 is a patch release to fix critical bugs. I don't think people are expecting tuning changes. v1.10 is only 1.5 months away anyway.

@LorenzMeier
Copy link
Member

@julianoes My comment was in relation to a different list you’re looking at now. However, we need to talk through a couple of these parameters - a broken configuration is as bad as a broken piece of code. What is clear is that the core dev team has been flying different defaults than were released, which makes this a 1.9 release bug. We should treat this as bad as an actual code issue, because the consequence for users is the same: The performance is worse than what has been demonstrated in pre-release QA testing.

@MaEtUgR
Copy link
Member

MaEtUgR commented Jul 16, 2019

Just wanted to mention the pr for UX parameter default changes here because it seems related: #12346

@jkflying
Copy link
Contributor

jkflying commented Jul 18, 2019

Another potential one is THR_MDL_FAC, it defaults to 0 which is a linear relationship between PWM and thrust, when the relationship is theoretically quadratic. A better value would be ~0.65, to take into account some linearities, and make the PIDs work correctly at all different thrust values.

Edit: actually, going back to when it was added, it was 0.65. It was set back to 0 in #6073 after flights with a QAV250, but no explanation why. I guess it made the throttle stick feel different in stabilized/acro? Maybe we then need an inverse function to apply to the throttle stick to keep the feel the same.

@julianoes julianoes moved this from Low priority to Currently under discussion in Devcall Jul 24, 2019
@julianoes julianoes moved this from Currently under discussion to Discussed in Devcall Jul 24, 2019
@julianoes
Copy link
Contributor

Notes from devcall:

  • @RomanBapst: there might be one more param for trigger interval
  • @RomanBapst: will be done when the PR is merged, presumably it will go into 1.10 since 1.10 should be released in 5 weeks already.

Release 1.11 Blockers automation moved this from To Do to Done Jul 29, 2019
Devcall automation moved this from Discussed to Done Jul 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

9 participants