diff --git a/firmware/controllers/can/can_listener.h b/firmware/controllers/can/can_listener.h index e7ad44229d..546c12ee3e 100644 --- a/firmware/controllers/can/can_listener.h +++ b/firmware/controllers/can/can_listener.h @@ -17,7 +17,7 @@ public: } CanListener* processFrame(const CANRxFrame& frame, efitick_t nowNt) { - if (CAN_ID(frame) == m_id) { + if (acceptFrame(frame)) { decodeFrame(frame, nowNt); } @@ -36,10 +36,17 @@ public: return m_next; } + // Return true if the provided frame should be accepted for processing by the listener. + // Override if you need more complex logic than comparing to a single ID. + virtual bool acceptFrame(const CANRxFrame& frame) const { + return CAN_ID(frame) == m_id; + } + protected: virtual void decodeFrame(const CANRxFrame& frame, efitick_t nowNt) = 0; - CanListener* m_next = nullptr; private: + CanListener* m_next = nullptr; + const uint32_t m_id; }; diff --git a/unit_tests/tests/test_can_rx.cpp b/unit_tests/tests/test_can_rx.cpp new file mode 100644 index 0000000000..021a9f93b4 --- /dev/null +++ b/unit_tests/tests/test_can_rx.cpp @@ -0,0 +1,67 @@ +#include "pch.h" + +#include "can_listener.h" + +using ::testing::StrictMock; +using ::testing::_; + +class MockCanListener : public CanListener { +public: + MockCanListener(uint32_t id) : CanListener(id) { } + + MOCK_METHOD(void, decodeFrame, (const CANRxFrame& frame, efitick_t nowNt), (override)); + MOCK_METHOD(bool, acceptFrame, (const CANRxFrame& frame), (const, override)); +}; + +TEST(CanListener, FrameAccepted) { + StrictMock dut(0x123); + + CANRxFrame frame; + + // Accept should be called, returns true + EXPECT_CALL(dut, acceptFrame(_)).WillOnce(Return(true)); + + // Because accept returns true, decode is called + EXPECT_CALL(dut, decodeFrame(_, 1234)); + + dut.processFrame(frame, 1234); +} + +TEST(CanListener, FrameNotAccepted) { + StrictMock dut(0x123); + + CANRxFrame frame; + + // Accept should be called, returns false, so decode not called + EXPECT_CALL(dut, acceptFrame(_)).WillOnce(Return(false)); + + dut.processFrame(frame, 1234); +} + +struct CanListenerNoDecode : public CanListener { + CanListenerNoDecode(uint32_t id) : CanListener(id) { } + + void decodeFrame(const CANRxFrame& frame, efitick_t nowNt) override { } +}; + +TEST(CanListener, FrameAcceptedChecksId) { + CanListenerNoDecode dut(0x123); + + CANRxFrame frame; + + frame.SID = 0x123; + frame.IDE = false; + + EXPECT_TRUE(dut.acceptFrame(frame)); +} + +TEST(CanListener, FrameNotAcceptedChecksId) { + CanListenerNoDecode dut(0x123); + + CANRxFrame frame; + + frame.SID = 0x456; + frame.IDE = false; + + EXPECT_FALSE(dut.acceptFrame(frame)); +} diff --git a/unit_tests/tests/tests.mk b/unit_tests/tests/tests.mk index cef5c53eda..bca6291a46 100644 --- a/unit_tests/tests/tests.mk +++ b/unit_tests/tests/tests.mk @@ -84,6 +84,7 @@ TESTS_SRC_CPP = \ tests/test_gpio.cpp \ tests/test_limp.cpp \ tests/trigger/test_all_triggers.cpp \ + tests/test_can_rx.cpp \ tests/test_can_serial.cpp \ tests/test_hellen_board_id.cpp \ tests/sensor/test_frequency_sensor.cpp \