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

commander: update parameter strings to class enum #13081

Merged
merged 2 commits into from
Oct 2, 2019
Merged

Conversation

dagar
Copy link
Member

@dagar dagar commented Oct 2, 2019

No description provided.

@dagar dagar force-pushed the pr-param_find_handle branch from 2befb3b to b67e396 Compare October 2, 2019 20:37
@dagar
Copy link
Member Author

dagar commented Oct 2, 2019

Saves a little bit of flash as well.

image

@dagar dagar force-pushed the pr-param_find_handle branch from b67e396 to 550549e Compare October 2, 2019 20:54
@dagar dagar merged commit 5351f88 into master Oct 2, 2019
@dagar dagar deleted the pr-param_find_handle branch October 2, 2019 23:43
@MaEtUgR
Copy link
Member

MaEtUgR commented Oct 3, 2019

This somehow breaks commander parameters. As a symptom I get a lot of errors in QGC even when just running SITL. It says COM_... parameters are missing and shows the vehicle needs setup before flying. Probably param_handle() does not replace param_find() and the parameters are not discovered and added to the list anymore?

I bisected and it only happens from c322f1d on.

@dagar
Copy link
Member Author

dagar commented Oct 3, 2019

Reverted for now.
a5895e5
f885d22

@BazookaJoe1900
Copy link
Member

@dagar - how about using param_set_used() in param_handle()?

@bkueng
Copy link
Member

bkueng commented Oct 7, 2019

@BazookaJoe1900 that API was not designed for that purpose. It's better to use the C++ classes (ParamFloat etc.)

@BazookaJoe1900
Copy link
Member

@bkueng the issue is that are about ~500 places with param_find("XXX"). and about 200 places where the parameter name build on real time. a better way is moving them to the c++ method, but that can be too hard to do.

more importantly here are spot that I found that try to read parameters that are not exist, on certain compilation:
camera_trigger/camera_trigger.cpp/275: _p_cam_cap_fback = param_find("CAM_CAP_FBACK"); linux_pwm_out/linux_pwm_out.cpp (4 matches)/144: param_t param_handle_mc_airmode = param_find("MC_AIRMODE"); linux_pwm_out/linux_pwm_out.cpp (4 matches)/558: param_get(param_find("PWM_DISARMED"), &linux_pwm_out::_pwm_disarmed); linux_pwm_out/linux_pwm_out.cpp (4 matches)/559: param_get(param_find("PWM_MIN"), &linux_pwm_out::_pwm_min); linux_pwm_out/linux_pwm_out.cpp (4 matches)/560: param_get(param_find("PWM_MAX"), &linux_pwm_out::_pwm_max); px4io/px4io.cpp (3 matches)/1,138: parm_handle = param_find("FW_MAN_R_SC"); px4io/px4io.cpp (3 matches)/1,145: parm_handle = param_find("FW_MAN_P_SC"); px4io/px4io.cpp (3 matches)/1,152: parm_handle = param_find("FW_MAN_Y_SC"); uavcanesc/uavcanesc_main.cpp (2 matches)/474: (void)param_get(param_find("ESC_NODE_ID"), &node_id); uavcanesc/uavcanesc_main.cpp (2 matches)/475: (void)param_get(param_find("ESC_BITRATE"), &bitrate); modules/camera_feedback/camera_feedback.cpp/60: _p_camera_capture_feedback = param_find("CAM_CAP_FBACK"); commander/Commander.cpp/1,240: param_t _param_airmode = param_find("MC_AIRMODE"); commander/PreflightCheck.cpp/918: param_get(param_find("FW_ARSP_MODE"), &optional); commander/rc_calibration.cpp (3 matches)/72: param_t param_fw_man_r_sc = param_find("FW_MAN_R_SC"); commander/rc_calibration.cpp (3 matches)/73: param_t param_fw_man_p_sc = param_find("FW_MAN_P_SC"); commander/rc_calibration.cpp (3 matches)/74: param_t param_fw_man_y_sc = param_find("FW_MAN_Y_SC"); fw_att_control/FixedwingAttitudeControl.cpp/382: if (param_get(param_find("VT_TYPE"), &vt_type) == PX4_OK) { fw_pos_control_l1/FixedwingPositionControl.cpp (2 matches)/345: _parameter_handles.airspeed_trans = param_find("VT_ARSP_TRANS"); fw_pos_control_l1/FixedwingPositionControl.cpp (2 matches)/346: _parameter_handles.vtol_type = param_find("VT_TYPE"); land_detector/MulticopterLandDetector.cpp (4 matches) land_detector/76: _paramHandle.landSpeed = param_find("MPC_LAND_SPEED"); land_detector/77: _paramHandle.minManThrottle = param_find("MPC_MANTHR_MIN"); land_detector/78: _paramHandle.minThrottle = param_find("MPC_THR_MIN"); land_detector/79: _paramHandle.hoverThrottle = param_find("MPC_THR_HOVER"); mc_att_control/mc_att_control_main.cpp/164: if (param_get(param_find("VT_TYPE"), &vt_type) == PX4_OK) { navigator/navigator_main.cpp (2 matches)/106: _handle_back_trans_dec_mss = param_find("VT_B_DEC_MSS"); navigator/navigator_main.cpp (2 matches)/107: _handle_reverse_delay = param_find("VT_B_REV_DEL"); precland.cpp (2 matches)/66: _handle_param_acceleration_hor = param_find("MPC_ACC_HOR"); precland.cpp (2 matches)/67: _handle_param_xy_vel_cruise = param_find("MPC_XY_CRUISE"); sensors/parameters.cpp (2 matches)/187: (void)param_find("TRIG_MODE"); sensors/parameters.cpp (2 matches)/188: (void)param_find("UAVCAN_ENABLE"); systemcmds/config/config.c (3 matches)/199: param_get(param_find("CAL_GYRO0_ID"), &(calibration_id)); systemcmds/config/config.c (3 matches)/253: param_get(param_find("CAL_MAG0_ID"), &(calibration_id)); systemcmds/config/config.c (3 matches)/297: param_get(param_find("CAL_ACC0_ID"), &(calibration_id));

@bkueng
Copy link
Member

bkueng commented Oct 11, 2019

@bkueng the issue is that are about ~500 places with param_find("XXX"). and about 200 places where the parameter name build on real time. a better way is moving them to the c++ method, but that can be too hard to do.

Yes that is why we have both interfaces. I don't see a problem with param_find at a few places. It's slower, but it has logarithmic runtime.

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.

4 participants