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

scripts: Fixed extract_features.py not extracting some features properly #27796

Merged
merged 2 commits into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Tools/scripts/build_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"), # noqa disabled because INERTIAL_LABS is the enable flag
Feature('AHRS', 'TEMPCAL', 'HAL_INS_TEMPERATURE_CAL_ENABLE', 'Enable IMU Temperature Calibration', 0, None),
Feature('AHRS', 'VISUALODOM', 'HAL_VISUALODOM_ENABLED', 'Enable Visual Odometry', 0, 'EKF3_EXTNAV'),
Feature('AHRS', 'EKF3_EXTNAV', 'EK3_FEATURE_EXTERNAL_NAV', 'Enable External Navigation for EKF3', 0, 'EKF3'),
Expand Down Expand Up @@ -169,7 +170,7 @@ def __init__(self,
Feature('Compass', 'QMC5883L', 'AP_COMPASS_QMC5883L_ENABLED', 'Enable QMC5883L compasses', 1, None),
Feature('Compass', 'RM3100', 'AP_COMPASS_RM3100_ENABLED', 'Enable RM3100 compasses', 1, None),
Feature('Compass', 'DRONECAN_COMPASS', 'AP_COMPASS_DRONECAN_ENABLED', 'Enable DroneCAN compasses', 0, "DroneCAN"),
Feature('Compass', 'DRONECAN_COMPASS_HIRES', 'AP_COMPASS_DRONECAN_HIRES_ENABLED', 'Enable DroneCAN HiRes compasses for survey logging', 0, "DroneCAN"), # noqa
Feature('Compass', 'DRONECAN_COMPASS_HIRES', 'AP_COMPASS_DRONECAN_HIRES_ENABLED', 'Enable DroneCAN HiRes compasses for survey logging', 0, "DroneCAN,DRONECAN_COMPASS"), # noqa
Feature('Compass', 'FixedYawCal', 'AP_COMPASS_CALIBRATION_FIXED_YAW_ENABLED', 'Enable Fixed-Yaw Compass Calibration', 1, None), # noqa
Feature('Compass', 'CompassLearn', 'COMPASS_LEARN_ENABLED', 'Enable In-Flight Compass Learning', 1, "FixedYawCal"),

Expand Down
9 changes: 5 additions & 4 deletions Tools/scripts/extract_features.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was not correctly detecting presence of ExternalAHRS_VectorNav

('HAL_INS_TEMPERATURE_CAL_ENABLE', 'AP_InertialSensor_TCal::Learn::save_calibration',),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was not detecting presence of Inertial Sensor TCal

Copy link
Contributor

Choose a reason for hiding this comment

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

Whoops.

('HAL_VISUALODOM_ENABLED', 'AP_VisualOdom::init',),

('AP_RANGEFINDER_ENABLED', 'RangeFinder::RangeFinder',),
('AP_RANGEFINDER_{type}_ENABLED', r'AP_RangeFinder_(?P<type>.*)::update\b',),
('AP_RANGEFINDER_{type}_ENABLED', r'AP_RangeFinder_(?P<type>.*)::get_reading\b',),
('AP_RANGEFINDER_{type}_ENABLED', r'AP_RangeFinder_(?P<type>.*)::model_dist_max_cm\b',),
('AP_RANGEFINDER_{type}_ENABLED', r'AP_RangeFinder_(?P<type>.*)::handle_frame\b',),
('AP_RANGEFINDER_LIGHTWARE_SERIAL_ENABLED', r'AP_RangeFinder_LightWareSerial::get_reading\b',),
('AP_RANGEFINDER_LWI2C_ENABLED', r'AP_RangeFinder_LightWareI2C::update\b',),
('AP_RANGEFINDER_MAXBOTIX_SERIAL_ENABLED', r'AP_RangeFinder_MaxsonarSerialLV::get_reading\b',),
Expand Down Expand Up @@ -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',),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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!

('AP_AVOIDANCE_ENABLED', 'AC_Avoid::AC_Avoid',),
('AP_OAPATHPLANNER_ENABLED', 'AP_OAPathPlanner::AP_OAPathPlanner',),
('AC_PAYLOAD_PLACE_ENABLED', 'PayloadPlace::start_descent'),
Expand Down Expand Up @@ -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'),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was not detecting properly. The class is Compass, not AP_Compass - perhaps Compass should be moved to AP_Compass at some point?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it should. I made that change for OpticalFlow, I think.

('COMPASS_LEARN_ENABLED', 'CompassLearn::update'),
('AP_CUSTOMROTATIONS_ENABLED', 'AP_CustomRotation::init'),
('AP_OSD_LINK_STATS_EXTENSIONS_ENABLED', r'AP_OSD_Screen::draw_rc_tx_power'),
Expand Down
Loading