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

rc_update: cleanup parameter usage #13672

Merged
merged 7 commits into from Dec 23, 2019
Merged

rc_update: cleanup parameter usage #13672

merged 7 commits into from Dec 23, 2019

Conversation

dagar
Copy link
Member

@dagar dagar commented Dec 4, 2019

No description provided.

Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

src/modules/rc_update/rc_update.cpp Outdated Show resolved Hide resolved
src/modules/rc_update/rc_update.cpp Outdated Show resolved Hide resolved
tmpScaleFactor = (1.0f / ((parameters.max[i] - parameters.min[i]) / 2.0f) * parameters.rev[i]);
tmpRevFactor = tmpScaleFactor * parameters.rev[i];

/* handle blowup in the scaling factor calculation */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You did not want to keep that check?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not opposed to keeping it, but not in this form printing an error message that's likely never going to be seen by a user. The scaling_factor it clears isn't even used anymore.

@dagar dagar requested a review from a team December 10, 2019 14:53
@dagar
Copy link
Member Author

dagar commented Dec 10, 2019

@PX4/testflights on any vehicle could you please try fully (re)calibrating RC and verify everything still works? If it checks out on the bench then please test fly and provide a log that can be compared to master. We can use that to compare calibration results before and after.

@Junkim3DR
Copy link

Tested on NXP FMUK66 v3

Modes Tested
Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.
Mission Plan Mode (Automated): Good.
RTL (Return To Land): Good.

Procedure

  • Calibrate RC before flying.
    Arm and Take off in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceed to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint then trigger RTL and see landing behavior.

Notes
No issues noted, good flight in general.

Log

Tested on Pixhawk 4 v5

Modes Tested
Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.
Mission Plan Mode (Automated): Good.
RTL (Return To Land): Good.

Procedure

  • Calibrate RC before flying.
    Arm and Take off in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceed to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint then trigger RTL and see landing behavior.

Notes
No issues noted, good flight in general.

Log

@jorge789
Copy link

Tested on PixRacer V4:

Modes Tested
Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.
Mission Plan Mode (Automated): Good.
RTL (Return To Land): Good.

Procedure

Calibrate RC before flying.
Arm and Take off in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceed to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint then trigger RTL and see landing behavior.
Notes
No issues noted, good flight in general.

Log

PR 13672
https://review.px4.io/plot_app?log=e6f4466b-2982-4ce9-ba50-0cc38ff76e5f

Master
https://review.px4.io/plot_app?log=56c74d70-5225-47a1-99e4-4ba2aaaacdbf

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.

None yet

5 participants