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

Move calibration from commander to event framework #8200

Closed
wants to merge 2 commits into from

Conversation

sugnanprabhu
Copy link
Contributor

This patch moves all calibration routines out of commander to events framework and removes commander_low_prio_loop.

Includes following set of changes.

  • On receiving calibration request
    i. commander does some minimal handling and move the arming state to ARMING_STATE_INIT.
    ii. event framework waits until the arming state is moved to ARMING_STATE_INIT before timeout of 2secs.
  • event framework spawns a px4 task to handle the calibration and make sure only one calibration is active.
  • event framework sends a calibration_status orb message after the completion of the calibration.
  • Commander will move the arming state back to ARMING_STATE_STANDBY once it receives calibration_status.

This is one of the part of task mentioned in #8014

@LorenzMeier
Copy link
Member

Cool, thanks! @bkueng @dagar More RAM!

This patch moves all the calibration routines out of commander to
events framework. Also removes low prio thread from commander.

Signed-off-by: Sugnan Prabhu S <sugnan.prabhu.s@intel.com>
Signed-off-by: Sugnan Prabhu S <sugnan.prabhu.s@intel.com>
@dagar
Copy link
Member

dagar commented Dec 12, 2017

Looks great! I resolved the conflicts, deleted some remaining low priority thread configuration, and rebased on px4 master. I'd like to get this in right after the v1.7.0 release.

@@ -237,9 +242,10 @@ int do_airspeed_calibration(orb_advert_t *mavlink_log_pub)
}

if (calibration_counter % 500 == 0) {
calibration_log_info(mavlink_log_pub, "[cal] Create air pressure! (got %d, wanted: 50 Pa)", (int)diff_pres.differential_pressure_filtered_pa);
tune_neutral(true);
Copy link
Member

@dagar dagar Dec 12, 2017

Choose a reason for hiding this comment

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

TODO - review calibration tunes

@@ -257,7 +263,6 @@ int do_airspeed_calibration(orb_advert_t *mavlink_log_pub)
calibration_log_info(mavlink_log_pub, CAL_QGC_PROGRESS_MSG, 100);

calibration_log_info(mavlink_log_pub, CAL_QGC_DONE_MSG, sensor_name);
tune_neutral(true);
Copy link
Member

Choose a reason for hiding this comment

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

TODO - review calibration tunes

@dagar
Copy link
Member

dagar commented Dec 12, 2017

TODO for @dagar

  • review commander stack requirements
  • review events stack requirements
  • tune usage (move to uORB?)
  • test everywhere (monitor memory usage)

@dagar
Copy link
Member

dagar commented Jan 8, 2018

@simonegu @LorenzMeier can we sync up this week to discuss plans for the tone_alarm uORB architecture? #7316

I'd strongly prefer getting tone usage via uORB in place first so that the calibration code isn't directly calling into the commander library in this PR. In fact we should also update the build system so that modules don't have free access to other modules like that (#8378).

@LorenzMeier
Copy link
Member

@sugnanprabhu Do you have time this week to rebase this? Ideally this would include porting the remaining parts of Commander into the (new) class architecture.

@LorenzMeier
Copy link
Member

I've rebased this here:
#8630

@sugnanprabhu Could you review if I did the rebase correctly? If not, please rebase yourself and I'll delete my branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants