scripts: Fixed extract_features.py not extracting some features properly #27796
Conversation
… Temp Cal properly
joshanne
force-pushed
the
pr/fix-extract-features
branch
from
234a15a to
07173e2
Compare
| @@ -73,14 +73,15 @@ def __init__(self, filename, nm="arm-none-eabi-nm", strings="strings"): | |||
| ('HAL_NAVEKF3_AVAILABLE', 'NavEKF3::NavEKF3',), | |||
| ('HAL_NAVEKF2_AVAILABLE', 'NavEKF2::NavEKF2',), | |||
| ('HAL_EXTERNAL_AHRS_ENABLED', r'AP_ExternalAHRS::init\b',), | |||
| ('AP_EXTERNAL_AHRS_{type}_ENABLED', r'AP_ExternalAHRS_{type}::healthy\b',), | |||
| ('HAL_INS_TEMPERATURE_CAL_ENABLE', 'AP_InertialSensor::TCal::Learn::save_calibration',), | |||
| ('AP_EXTERNAL_AHRS_{type}_ENABLED', r'AP_ExternalAHRS_(?P<type>.*)::healthy\b',), | |||
There was a problem hiding this comment.
This was not correctly detecting presence of ExternalAHRS_VectorNav
| ('AP_EXTERNAL_AHRS_{type}_ENABLED', r'AP_ExternalAHRS_{type}::healthy\b',), | ||
| ('HAL_INS_TEMPERATURE_CAL_ENABLE', 'AP_InertialSensor::TCal::Learn::save_calibration',), | ||
| ('AP_EXTERNAL_AHRS_{type}_ENABLED', r'AP_ExternalAHRS_(?P<type>.*)::healthy\b',), | ||
| ('HAL_INS_TEMPERATURE_CAL_ENABLE', 'AP_InertialSensor_TCal::Learn::save_calibration',), |
There was a problem hiding this comment.
This was not detecting presence of Inertial Sensor TCal
| @@ -142,7 +143,7 @@ def __init__(self, filename, nm="arm-none-eabi-nm", strings="strings"): | |||
|
|
|||
| ('HAL_PARACHUTE_ENABLED', 'AP_Parachute::update',), | |||
| ('AP_FENCE_ENABLED', r'AC_Fence::check\b',), | |||
| ('HAL_RALLY_ENABLED', r'AP_Rally::get_rally_max\b',), | |||
| ('HAL_RALLY_ENABLED', 'AP_Rally::find_nearest_rally_point',), | |||
There was a problem hiding this comment.
I noticed Rally was not being detected by default? get_rally_max is not showing when running arm-none-eabi-nm --demangle --print-size - so I've moved this to a function that is visible and used.
There was a problem hiding this comment.
Compiler was probably inlining; you could try recompiling with a different optimisation level to see if that's true. But it can't possibly be being elided given where it is used!
| @@ -257,7 +258,7 @@ def __init__(self, filename, nm="arm-none-eabi-nm", strings="strings"): | |||
| ('FORCE_APJ_DEFAULT_PARAMETERS', 'AP_Param::param_defaults_data'), | |||
| ('HAL_BUTTON_ENABLED', 'AP_Button::update'), | |||
| ('HAL_LOGGING_ENABLED', 'AP_Logger::init'), | |||
| ('AP_COMPASS_CALIBRATION_FIXED_YAW_ENABLED', 'AP_Compass::mag_cal_fixed_yaw'), | |||
| ('AP_COMPASS_CALIBRATION_FIXED_YAW_ENABLED', 'Compass::mag_cal_fixed_yaw'), | |||
There was a problem hiding this comment.
This was not detecting properly. The class is Compass, not AP_Compass - perhaps Compass should be moved to AP_Compass at some point?
There was a problem hiding this comment.
Yes, it should. I made that change for OpticalFlow, I think.
Tools/scripts/build_options.py
Outdated
| @@ -34,6 +34,7 @@ def __init__(self, | |||
| Feature('AHRS', 'MicroStrain5', 'AP_EXTERNAL_AHRS_MICROSTRAIN5_ENABLED', 'Enable MICROSTRAIN 5-series External AHRS', 0, "AHRS_EXT"), # noqa: E501 | |||
| Feature('AHRS', 'MicroStrain7', 'AP_EXTERNAL_AHRS_MICROSTRAIN7_ENABLED', 'Enable MICROSTRAIN 7-series External AHRS', 0, "AHRS_EXT"), # noqa: E501 | |||
| Feature('AHRS', 'AHRS_EXT_VECTORNAV', 'AP_EXTERNAL_AHRS_VECTORNAV_ENABLED', 'Enable VectorNav External AHRS', 0, "AHRS_EXT"), # noqa | |||
| #Feature('AHRS', 'InertialLabs', 'AP_EXTERNAL_AHRS_INERTIAL_LABS_ENABLED', 'Enable InertialLabs External AHRS', 0, "AHRS_EXT"), # disabled because INERTIAL_LABS is the enable flag | |||
There was a problem hiding this comment.
There are 9 instances of AP_EXTERNAL_AHRS_INERTIAL_LABS_ENABLED in the code base - I'd like to change them to AP_EXTERNAL_AHRS_INERTIALLABS_ENABLED so this line can be uncommented and changed correctly.
We can either detect the InertialLabs class correctly, but not use this build_option feature externally, OR, incorrectly detect the feature, but correctly use the build_options to generate a file.
What should it be?
There was a problem hiding this comment.
Of course, this would potentially break peoples custom build servers...
Could rename the class.
Is there a way to map extracted feature classes to a different define name? I couldn't see one
There was a problem hiding this comment.
I'm fine with this rename
There was a problem hiding this comment.
Excellent. I'll push a patch for that shortly.
The enable flag is AP_EXTERNAL_AHRS_INERTIAL_LABS_ENABLED, but the extract features can only generate based on the class name, which is InertialLabs.
joshanne
force-pushed
the
pr/fix-extract-features
branch
from
07173e2 to
9e0ed3e
Compare
joshanne
changed the title
|
Thanks for this. I've just written patches here that allow |
|
Merged, thanks! |
extract_features.pyis not detecting/correctly extracting:Tested against a local build of CubeOrangePlus with hwdef modifications disabling the feature.