Skip to content

Commit

Permalink
[BLDC] Only enable rotor offset compensation when hall interpolation …
Browse files Browse the repository at this point in the history
…is enabled

The commit enabling sector offset compensation accidentally made some poor unit tests wherein the raw hall angle was not passed through if interpolation was disabled.

This feature should only be active if interpolation is also active, otherwise it does not make sense to actually enable this feature.

Fix adds a param flag for this and also fixes the unit tests to ensure raw hall angle passthrough
  • Loading branch information
sahil-kale committed Dec 31, 2023
1 parent 6634120 commit 7468ded
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 6 deletions.
2 changes: 1 addition & 1 deletion control_loop/bldc/rotor_estimator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ app_hal_status_E ElectricalRotorPosEstimatorFromHall::update(const ElectricalRot
// adding or subtracting 30 degrees from the rotor position estimate depending on the
// sign of the velocity. This will make the rotor position estimate more accurate

if (params_->enable_sector_position_offset_compensation) {
if (params_->enable_sector_position_offset_compensation && params_->enable_interpolation) {
if (velocity_ > 0.0f) {
rotor_position_ -= math::M_PI_FLOAT / 6.0f;
} else {
Expand Down
9 changes: 4 additions & 5 deletions test/rotor_estimator_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ TEST(RotorEstimatorTest, test_angle_interpolation_disabled) {
float rotor_position = 0.0f;
rotor_estimator.get_rotor_position(rotor_position);

EXPECT_FLOAT_EQ(rotor_position, 0.5f * 2.0f * M_PI / 6.0f);
EXPECT_FLOAT_EQ(rotor_position, 1 * math::M_PI_FLOAT / 3.0);

EXPECT_CALL(sector_sensor, get_electrical_angle(_)) // _ allowing any param
.WillOnce(DoAll(SetArgReferee<0>(1 * math::M_PI_FLOAT / 3.0), Return(APP_HAL_OK)));
Expand All @@ -169,7 +169,7 @@ TEST(RotorEstimatorTest, test_sector_position_offset_compensation) {
bldc_rotor_estimator::ElectricalRotorPosEstimatorFromHall::ElectricalRotorPosEstimatorFromHallParams params{
.num_hall_updates_to_start = 10,
.max_estimate_angle_overrun = 2.0f / 3.0f * M_PI,
.enable_interpolation = false,
.enable_interpolation = true,
.enable_sector_position_offset_compensation = true,
.minimum_estimation_velocity = 0.0f,
};
Expand Down Expand Up @@ -223,7 +223,7 @@ TEST(RotorEstimatorTest, test_sector_position_offset_compensation_disabled) {
.num_hall_updates_to_start = 10,
.max_estimate_angle_overrun = 2.0f / 3.0f * M_PI,
.enable_interpolation = false,
.enable_sector_position_offset_compensation = true,
.enable_sector_position_offset_compensation = false,
.minimum_estimation_velocity = 0.0f,
};

Expand Down Expand Up @@ -260,8 +260,7 @@ TEST(RotorEstimatorTest, test_sector_position_offset_compensation_disabled) {
// Get the rotor position
rotor_estimator.get_rotor_position(rotor_position);

// Expect the rotor position to be 11pi/6
EXPECT_FLOAT_EQ(rotor_position, 11.0f * math::M_PI_FLOAT / 6.0f);
EXPECT_FLOAT_EQ(rotor_position, 10.0f * math::M_PI_FLOAT / 6.0f);
}

// Test that with a position offset compensation flag of 1 with a max angle tolerance of 2PI/3 when
Expand Down

0 comments on commit 7468ded

Please sign in to comment.