Fix lack of test coverage for angle_meas scaling in subaru.h

[jacobwaller]

Validation

• Dongle ID:
• Route:

[github-actions]

Testing 79 segments for: SUBARU_ASCENT, SUBARU_OUTBACK, SUBARU_LEGACY, SUBARU_IMPREZA, SUBARU_IMPREZA_2020, SUBARU_FORESTER, SUBARU_OUTBACK_PREGLOBAL, SUBARU_OUTBACK_PREGLOBAL_2018, SUBARU_FORESTER_2022

Results: 79 passed, 0 with diffs, 0 errors

[jacobwaller]

This change is not needed, but is there to demonstrate that the tests added cover the mutant tests for this line

Feel free to accept this suggestion to undo the change:

Suggested change

	angle_meas_new = ROUND(-2.17 * to_signed(angle_meas_new, 16));
	angle_meas_new = ROUND(to_signed(angle_meas_new, 16) * -2.17);

[sshane]

Don't we have a test to make sure angle_meas_new is parsed properly in common.py?

[jacobwaller]

I'm not really sure if that's the intention of those tests as they don't catch the mutation:
Changing only one line to trigger a diff in opendbc master:

❯ git diff
diff --git a/opendbc/safety/modes/subaru.h b/opendbc/safety/modes/subaru.h
index 9b01d39d..6871cf06 100644
--- a/opendbc/safety/modes/subaru.h
+++ b/opendbc/safety/modes/subaru.h
@@ -101,7 +101,7 @@ static void subaru_rx_hook(const CANPacket_t *msg) {
 
     int angle_meas_new = (GET_BYTES(msg, 4, 2) & 0xFFFFU);
     // convert Steering_Torque -> Steering_Angle to centidegrees, to match the ES_LKAS_ANGLE angle request units
-    angle_meas_new = ROUND(to_signed(angle_meas_new, 16) * -2.17);
+    angle_meas_new = ROUND(-2.17 * to_signed(angle_meas_new, 16));
     update_sample(&angle_meas, angle_meas_new);
   }

Results in the mutation tests failing as before:

/Users/jacobwaller/Documents/personal/comma/opendbc/opendbc/safety/modes/subaru.h:104:34: warning: Survived: Replaced * with / [cxx_mul_to_div]
    angle_meas_new = ROUND(-2.17 * to_signed(angle_meas_new, 16));
                                 ^

[jacobwaller]

Oh I see, that steering angle test is only being done for angle-based cars

class AngleSteeringSafetyTest(VehicleSpeedSafetyTest):

... other code

  def test_steering_angle_measurements(self):
    self._common_measurement_test(self._angle_meas_msg, -self.STEER_ANGLE_MAX, self.STEER_ANGLE_MAX, self.DEG_TO_CAN,
                                  self.safety.get_angle_meas_min, self.safety.get_angle_meas_max)

[jacobwaller]

Alright @sshane I updated to use the helper in common.py

One note -- I wanted to just add this to all the cars, but not everything has an easily accessible STEER_ANGLE_MAX or a way to grab the angle_meas. In fact, from what I can see, some cars don't even have angle_meas in safety (hyundai.h for example)? I might be missing something there though.

As a workaround, I just added it in test_subaru.py

[sshane]

If the safety test/mode is for angle, it should subclass from that base angle test

[jacobwaller]

The case we're looking at here is for torque-based, I'm hesitant to subclass from that because subarus don't support angle based steering yet.

[jacobwaller]

Hey @sshane , sorry to ping you again :). I refactored the scaling check into a dedicated test (AngleScalingSafetyTest) and made both AngleSteeringSafetyTest and TestSubaruSafetyBase inherit from it.

This should keep these tests a little more DRY, let me know if this is kind of what you were thinking

[sshane]

I see we merged a PR to add angle-based signals before we actually added angle safety... commaai/panda#1528 Then we never merged commaai/panda#1533

[sshane]

AngleSteeringSafetyTest should catch this in #2864. I'll check out why it doesn't