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

AUX outputs param setup fix #5990

Closed
wants to merge 9 commits into from
Closed

AUX outputs param setup fix #5990

wants to merge 9 commits into from

Conversation

LorenzMeier
Copy link
Member

No description provided.

@LorenzMeier
Copy link
Member Author

Flash overflow. So we're just at the limit.

@LorenzMeier
Copy link
Member Author

@tops4u It would be great if you could test this.

@tops4u
Copy link
Contributor

tops4u commented Dec 12, 2016

@LorenzMeier Checked and failed : See #5989

@LorenzMeier
Copy link
Member Author

LorenzMeier commented Dec 12, 2016

@tops4u This works now, I just tested it.

Paramter PWM_AUX_DISARMED set to 1490 and rebooted:

pwm info -d /dev/pwm_output1
device: /dev/pwm_output1
channel 1: 1490 us (alternative rate: 50 Hz failsafe: 0, disarmed: 1490 us, min: 1000 us, max: 2000 us, trim:  0.00)
channel 2: 1490 us (alternative rate: 50 Hz failsafe: 0, disarmed: 1490 us, min: 1000 us, max: 2000 us, trim:  0.00)
channel 3: 1490 us (alternative rate: 50 Hz failsafe: 0, disarmed: 1490 us, min: 1000 us, max: 2000 us, trim:  0.00)
channel 4: 1490 us (alternative rate: 50 Hz failsafe: 0, disarmed: 1490 us, min: 1000 us, max: 2000 us, trim:  0.00)
channel 5: 1 us (default rate: 50 Hz failsafe: 0, disarmed: 0 us, min: 1000 us, max: 2000 us, trim:  0.00)
channel 6: 1 us (default rate: 50 Hz failsafe: 0, disarmed: 0 us, min: 1000 us, max: 2000 us, trim:  0.00)
channel group 0: channels 1 2 3 4
channel group 1: channels 5 6

@LorenzMeier
Copy link
Member Author

@julianoes Can you briefly review?

@LorenzMeier LorenzMeier added this to the Release v2.0.0 milestone Dec 12, 2016
@LorenzMeier LorenzMeier requested review from kd0aij and removed request for julianoes December 12, 2016 19:01
@LorenzMeier
Copy link
Member Author

@kd0aij Can you review and maybe test this one?

@@ -122,7 +122,7 @@ get_parameter_value(const char *option, const char *paramDescription)
if (strncmp("p:", option, 2) == 0) {

char paramName[32];
strncpy(paramName, option + 2, 16);
strncpy(paramName, option + 2, 17);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is 17 the maximum length of a parameter name?
Everything else looks good to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Param name + 1 (null termination). See: http://pubs.opengroup.org/onlinepubs/7908799/xsh/strncpy.html.

Copy link
Contributor

@kd0aij kd0aij Dec 12, 2016

Choose a reason for hiding this comment

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

I knew that exactly 17 bytes should be written to paramName, but I couldn't find any guarantee that option points to a string with length < 17
Interesting that PWM_AUX_DISARMED is 16 bytes long.

Dusted off the Pixhawk1 F450; AUX outputs working, as is the default aux_disarmed parameter. Will do a short flight test next.

Copy link
Contributor

Choose a reason for hiding this comment

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

For future reference, the parameter name length limit of 16 characters is enforced in src/Tools/px4params/srcparser.py

@LorenzMeier
Copy link
Member Author

@kd0aij Any chance you could flash & take this for a short flight?

@kd0aij
Copy link
Contributor

kd0aij commented Dec 13, 2016

Test flight went OK.

For some reason I had to recalibrate the ESCs before this would fly again, and I ran out of battery after just a minute in the air, but performance in stabilized and altctl with EKF2 was nominal.
I had changed the fmu-v2 config to compile the new logger, but managed to lose the sys_logger parameter change, so no logger was running. Will do a longer flight tomorrow with the new logger.

@LorenzMeier
Copy link
Member Author

Thanks for the baseline testing. Rebased and applied.

@LorenzMeier LorenzMeier deleted the aux_param_fix branch December 13, 2016 08:19
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

4 participants