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

VehicleIMU: param change of SENS_IMU_CLPNOTI should have effect without reboot #23912

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

sfuhrer
Copy link
Contributor

@sfuhrer sfuhrer commented Nov 8, 2024

Solved Problem

A change of SENS_IMU_CLPNOTI required a reboot. I would expect it to have effect immediately (there is no @reboot required in the meta data).

Solution

Set _notify_clipping on parameter update and not in constructor.

Changelog Entry

For release notes:

Bugfix VehicleIMU: param change of SENS_IMU_CLPNOTI should have effect without reboot

@sfuhrer sfuhrer requested a review from KonradRudin November 8, 2024 09:41
_accel_calibration.ParametersUpdate();
_gyro_calibration.ParametersUpdate();

const auto accel_calibration_count = _accel_calibration.calibration_count();
Copy link
Member

Choose a reason for hiding this comment

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

Why did this move? It's need to grab the calibration count before the accel calibration parameters are updated, then we know below if it actually changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true, seems I was a bit overeager and didn't take enough time to understand what's going on.

Reverted it.

@sfuhrer sfuhrer force-pushed the pr-vehicle-imu-fix-clipping-noti-param-update-main branch from 9be2b9e to d87f99d Compare November 11, 2024 08:24
@dagar dagar merged commit f9c4c8b into main Nov 13, 2024
54 of 56 checks passed
@dagar dagar deleted the pr-vehicle-imu-fix-clipping-noti-param-update-main branch November 13, 2024 17:10
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.

3 participants