Only run tx hooks on msgs in safety_config.tx_msgs.

[ccdunder]

#1838

Dependencies: #1867

TODO:

• write a test for this
• file an issue to track backfilling the missing hyundai_canfd test

[sshane]

safety tests are failing

[sshane]

Are our tests wrong?

[ccdunder]

tests are complaining about coverage for lines that no longer get hit.

also i got stuck in a bad state last night that was only recoverable by clearing scons cache. ugh

[ccdunder]

@sshane nooutput tx hook will never get exercised after this change. i'm going to exclude the line from coverage and misra and document it (though technically the misra checker didn't catch it anyway). if that's not acceptable for misra, let me know if there's an approach you'd generally take for this kind of thing. (i could always concoct a "test" to exercise that line but that seems convoluted.)

also, just to check, does tx hook should be filtered by address mean ignore length? (my guess is no but figured i'd ask in case you see this before the final code review)

[ccdunder]

Just want to call out that it's certainly possible that some car models rely on the existing bad behavior. What's your rollout policy for potentially breaking changes?

1. are harness failures acceptable in master? (anticipating they get reported before release)
2. do you have a way to run this over all recent car segments to check for a regression?
3. otherwise, it could be done as a dark launch. my hunch is this is overkill, but really can't say without knowing your policy.

[ccdunder]

also this seems like something that should really have a test but there doesn't seem to be a simple way to do right now with adding new test infra. i'm assuming (& hoping) you want that in the same PR, but just calling out that it will make it larger.

Just to be clear, I believe making it larger is the right thing to do based on my principles as follow. Please let me know if you require something different:

i'm not backfilling existing missing tests (in hyundai_canfd) (since they're already missing so this strictly improves the codebase regardless). i am adding a test for the behavior this modifies (since not doing so strictly worsens the codebase by making bugs like this possible).

[ccdunder]

TIL the coverage report uses a different tool (lcov) than the coverage test (gcov). gcov doesn't support exclusion markers easily. gcovr does. will send a separate PR to add support for exclusion from gcov.

[ccdunder]

safety tests pass with #1867. will figure out some decent way of testing this behavior next

[sshane]

Are you sure our test isn't accidentally sending this on the wrong bus? We should definitely hit this

[sshane]

Is this because 0x160 is now blocked since it's not in the tx messages?

[ccdunder]

correct. the case where addr == 0x160 && !rivian_longitudinal becomes unreachable with this change.