From 2cb5e821b4865cf4d8e0c717d3ec10cd4f25343f Mon Sep 17 00:00:00 2001 From: t0mpr1c3 Date: Wed, 4 Oct 2023 07:46:35 -0400 Subject: [PATCH] remove OpKnit::encodePosition() --- src/ayab/com.cpp | 2 +- src/ayab/com.h | 2 +- src/ayab/controller.cpp | 21 ++++++++++++++------- src/ayab/controller.h | 31 +++++++++++++++++-------------- src/ayab/global_OpKnit.cpp | 4 ---- src/ayab/global_com.cpp | 2 -- src/ayab/global_controller.cpp | 4 ++++ src/ayab/opKnit.cpp | 16 ---------------- src/ayab/opKnit.h | 4 ---- src/ayab/opTest.cpp | 1 - test/mocks/controller_mock.cpp | 19 ++++++++++++------- test/mocks/controller_mock.h | 15 ++++++++------- test/mocks/opKnit_mock.cpp | 5 ----- test/mocks/opKnit_mock.h | 1 - test/test_OpKnit.cpp | 13 ------------- test/test_OpTest.cpp | 8 -------- test/test_com.cpp | 2 ++ test/test_controller.cpp | 9 +++++++++ 18 files changed, 68 insertions(+), 91 deletions(-) diff --git a/src/ayab/com.cpp b/src/ayab/com.cpp index 1bd6e4783..bfc15dd5d 100644 --- a/src/ayab/com.cpp +++ b/src/ayab/com.cpp @@ -158,7 +158,7 @@ void Com::send_indState(Err_t error) const { * \param size The number of bytes in the data buffer. */ void Com::onPacketReceived(const uint8_t *buffer, size_t size) { - GlobalController::getState()->com(buffer, size); + GlobalController::com(buffer, size); } // Serial command handling diff --git a/src/ayab/com.h b/src/ayab/com.h index 0474dd070..615a653dd 100644 --- a/src/ayab/com.h +++ b/src/ayab/com.h @@ -138,7 +138,7 @@ class Com : public ComInterface { public: void init() final; void update() final; - uint8_t CRC8(const uint8_t *buffer, size_t len) const; + uint8_t CRC8(const uint8_t *buffer, size_t len) const final; void send(uint8_t *payload, size_t length) const final; void sendMsg(API_t id, const char *msg) final; void sendMsg(API_t id, char *msg) final; diff --git a/src/ayab/controller.cpp b/src/ayab/controller.cpp index fc8fdd909..5321d6df3 100644 --- a/src/ayab/controller.cpp +++ b/src/ayab/controller.cpp @@ -74,6 +74,13 @@ void Controller::update() { m_currentState = m_nextState; } +/*! + * \brief Call communication method for current machine state + */ +void Controller::com(const uint8_t *buffer, size_t size) const { + m_currentState->com(buffer, size); +} + /*! * \brief Cache Encoder values * The code that saves the Encoder values is bookended by macros @@ -109,7 +116,7 @@ void Controller::setState(OpInterface *state) { * \brief Get machine state. * \return Current state of Finite State Machine. */ -OpInterface *Controller::getState() { +OpInterface *Controller::getState() const { return m_currentState; } @@ -125,7 +132,7 @@ void Controller::setMachineType(Machine_t machineType) { * \brief Get knitting machine type. * \return Machine type. */ -Machine_t Controller::getMachineType() { +Machine_t Controller::getMachineType() const { return m_machineType; } @@ -133,7 +140,7 @@ Machine_t Controller::getMachineType() { * \brief Get cached beltShift value. * \return Cached beltShift value. */ -BeltShift_t Controller::getBeltShift() { +BeltShift_t Controller::getBeltShift() const { return m_beltShift; } @@ -141,7 +148,7 @@ BeltShift_t Controller::getBeltShift() { * \brief Get cached carriage value. * \return Cached carriage value. */ -Carriage_t Controller::getCarriage() { +Carriage_t Controller::getCarriage() const { return m_carriage; } @@ -149,7 +156,7 @@ Carriage_t Controller::getCarriage() { * \brief Get cached direction value. * \return Cached direction value. */ -Direction_t Controller::getDirection() { +Direction_t Controller::getDirection() const { return m_direction; } @@ -157,7 +164,7 @@ Direction_t Controller::getDirection() { * \brief Get cached hallActive value. * \return Cached hallActive value. */ -Direction_t Controller::getHallActive() { +Direction_t Controller::getHallActive() const { return m_hallActive; } @@ -165,6 +172,6 @@ Direction_t Controller::getHallActive() { * \brief Get cached position value. * \return Cached position value. */ -uint8_t Controller::getPosition() { +uint8_t Controller::getPosition() const { return m_position; } diff --git a/src/ayab/controller.h b/src/ayab/controller.h index 2b9d0912b..6b102f7d4 100644 --- a/src/ayab/controller.h +++ b/src/ayab/controller.h @@ -36,16 +36,17 @@ class ControllerInterface { // any methods that need to be mocked should go here virtual void init() = 0; virtual void update() = 0; + virtual void com(const uint8_t *buffer, size_t size) const = 0; virtual void cacheEncoders() = 0; virtual void setState(OpInterface *state) = 0; - virtual OpInterface *getState() = 0; + virtual OpInterface *getState() const = 0; virtual void setMachineType(Machine_t) = 0; - virtual Machine_t getMachineType() = 0; - virtual BeltShift_t getBeltShift() = 0; - virtual Carriage_t getCarriage() = 0; - virtual Direction_t getDirection() = 0; - virtual Direction_t getHallActive() = 0; - virtual uint8_t getPosition() = 0; + virtual Machine_t getMachineType() const = 0; + virtual BeltShift_t getBeltShift() const = 0; + virtual Carriage_t getCarriage() const = 0; + virtual Direction_t getDirection() const = 0; + virtual Direction_t getHallActive() const = 0; + virtual uint8_t getPosition() const = 0; }; // Singleton container class for static methods. @@ -65,6 +66,7 @@ class GlobalController final { static void init(); static void update(); + static void com(const uint8_t *buffer, size_t size); static void cacheEncoders(); static void setState(OpInterface *state); static OpInterface *getState(); @@ -81,16 +83,17 @@ class Controller : public ControllerInterface { public: void init() final; void update() final; + void com(const uint8_t *buffer, size_t size) const final; void cacheEncoders() final; void setState(OpInterface *state) final; - OpInterface *getState() final; + OpInterface *getState() const final; void setMachineType(Machine_t) final; - Machine_t getMachineType() final; - BeltShift_t getBeltShift() final; - Carriage_t getCarriage() final; - Direction_t getDirection() final; - Direction_t getHallActive() final; - uint8_t getPosition() final; + Machine_t getMachineType() const final; + BeltShift_t getBeltShift() const final; + Carriage_t getCarriage() const final; + Direction_t getDirection() const final; + Direction_t getHallActive() const final; + uint8_t getPosition() const final; // machine state OpInterface *m_currentState; diff --git a/src/ayab/global_OpKnit.cpp b/src/ayab/global_OpKnit.cpp index 467c7ea4d..0f4a9c1ed 100644 --- a/src/ayab/global_OpKnit.cpp +++ b/src/ayab/global_OpKnit.cpp @@ -59,10 +59,6 @@ Err_t GlobalOpKnit::startKnitting(uint8_t startNeedle, pattern_start, continuousReportingEnabled); } -void GlobalOpKnit::encodePosition() { - m_instance->encodePosition(); -} - void GlobalOpKnit::knit() { m_instance->knit(); } diff --git a/src/ayab/global_com.cpp b/src/ayab/global_com.cpp index 302f28eab..9d2f4f8d1 100644 --- a/src/ayab/global_com.cpp +++ b/src/ayab/global_com.cpp @@ -58,11 +58,9 @@ void GlobalCom::send_indState(Err_t error) { m_instance->send_indState(error); } -// GCOVR_EXCL_START void GlobalCom::onPacketReceived(const uint8_t *buffer, size_t size) { m_instance->onPacketReceived(buffer, size); } -// GCOVR_EXCL_STOP void GlobalCom::h_reqInit(const uint8_t *buffer, size_t size) { m_instance->h_reqInit(buffer, size); diff --git a/src/ayab/global_controller.cpp b/src/ayab/global_controller.cpp index 49297c5f8..096fdd5e4 100644 --- a/src/ayab/global_controller.cpp +++ b/src/ayab/global_controller.cpp @@ -32,6 +32,10 @@ void GlobalController::update() { m_instance->update(); } +void GlobalController::com(const uint8_t *buffer, size_t size) { + m_instance->com(buffer, size); +} + void GlobalController::cacheEncoders() { m_instance->cacheEncoders(); } diff --git a/src/ayab/opKnit.cpp b/src/ayab/opKnit.cpp index 2412a6475..dce52cf00 100644 --- a/src/ayab/opKnit.cpp +++ b/src/ayab/opKnit.cpp @@ -167,22 +167,6 @@ Err_t OpKnit::startKnitting(uint8_t startNeedle, return Err_t::Success; } -/*! - * \brief Record current encoder position. - * - * Used in hardware test procedure. - */ -void OpKnit::encodePosition() { - auto position = GlobalController::getPosition(); - if (m_sOldPosition != position) { - // only act if there is an actual change of position - // store current encoder position for next call of this function - m_sOldPosition = position; - (void) calculatePixelAndSolenoid(); - GlobalCom::send_indState(Err_t::Unspecified_failure); // FIXME is this the right error code? - } -} - /*! * \brief Function that is repeatedly called during state `OpKnit` */ diff --git a/src/ayab/opKnit.h b/src/ayab/opKnit.h index 04a080674..284179f87 100644 --- a/src/ayab/opKnit.h +++ b/src/ayab/opKnit.h @@ -35,7 +35,6 @@ class OpKnitInterface : public OpInterface { virtual Err_t startKnitting(uint8_t startNeedle, uint8_t stopNeedle, uint8_t *pattern_start, bool continuousReportingEnabled) = 0; - virtual void encodePosition() = 0; virtual void knit() = 0; virtual uint8_t getStartOffset(const Direction_t direction) = 0; virtual bool setNextLine(uint8_t lineNumber) = 0; @@ -67,7 +66,6 @@ class GlobalOpKnit final { static Err_t startKnitting(uint8_t startNeedle, uint8_t stopNeedle, uint8_t *pattern_start, bool continuousReportingEnabled); - static void encodePosition(); static void knit(); static uint8_t getStartOffset(const Direction_t direction); static bool setNextLine(uint8_t lineNumber); @@ -86,7 +84,6 @@ class OpKnit : public OpKnitInterface { Err_t startKnitting(uint8_t startNeedle, uint8_t stopNeedle, uint8_t *pattern_start, bool continuousReportingEnabled) final; - void encodePosition() final; void knit() final; uint8_t getStartOffset(const Direction_t direction) final; bool setNextLine(uint8_t lineNumber) final; @@ -120,7 +117,6 @@ class OpKnit : public OpKnitInterface { #if AYAB_TESTS // Note: ideally tests would only rely on the public interface. - FRIEND_TEST(OpKnitTest, test_encodePosition); FRIEND_TEST(OpKnitTest, test_getStartOffset); FRIEND_TEST(OpKnitTest, test_knit_lastLine_and_no_req); FRIEND_TEST(OpKnitTest, test_calculatePixelAndSolenoid); diff --git a/src/ayab/opTest.cpp b/src/ayab/opTest.cpp index 9e03c7173..9e69a8c3c 100644 --- a/src/ayab/opTest.cpp +++ b/src/ayab/opTest.cpp @@ -85,7 +85,6 @@ void OpTest::begin() { */ void OpTest::update() { if (enabled()) { - GlobalOpKnit::encodePosition(); // FIXME is this even necessary? uint32_t now = millis(); if (now - m_lastTime >= TEST_LOOP_DELAY) { m_lastTime = now; diff --git a/test/mocks/controller_mock.cpp b/test/mocks/controller_mock.cpp index 886fb018a..a295c810d 100644 --- a/test/mocks/controller_mock.cpp +++ b/test/mocks/controller_mock.cpp @@ -50,6 +50,11 @@ void Controller::update() { gControllerMock->update(); } +uint8_t Controller::com(const uint8_t *buffer, size_t size) const { + assert(gControllerMock != nullptr); + return gControllerMock->com(buffer, size); +} + void Controller::cacheEncoders() { assert(gControllerMock != nullptr); gControllerMock->cacheEncoders(); @@ -60,7 +65,7 @@ void Controller::setState(OpInterface *state) { gControllerMock->setState(state); } -OpInterface *Controller::getState() { +OpInterface *Controller::getState() const { assert(gControllerMock != nullptr); return gControllerMock->getState(); } @@ -70,32 +75,32 @@ void Controller::setMachineType(Machine_t machineType) { gControllerMock->setMachineType(machineType); } -Machine_t Controller::getMachineType() { +Machine_t Controller::getMachineType() const { assert(gControllerMock != nullptr); return gControllerMock->getMachineType(); } -BeltShift_t Controller::getBeltShift() { +BeltShift_t Controller::getBeltShift() const { assert(gControllerMock != nullptr); return gControllerMock->getBeltShift(); } -Carriage_t Controller::getCarriage() { +Carriage_t Controller::getCarriage() const { assert(gControllerMock != nullptr); return gControllerMock->getCarriage(); } -Direction_t Controller::getDirection() { +Direction_t Controller::getDirection() const { assert(gControllerMock != nullptr); return gControllerMock->getDirection(); } -Direction_t Controller::getHallActive() { +Direction_t Controller::getHallActive() const { assert(gControllerMock != nullptr); return gControllerMock->getHallActive(); } -uint8_t Controller::getPosition() { +uint8_t Controller::getPosition() const { assert(gControllerMock != nullptr); return gControllerMock->getPosition(); } diff --git a/test/mocks/controller_mock.h b/test/mocks/controller_mock.h index 1afd15e47..4301120b4 100644 --- a/test/mocks/controller_mock.h +++ b/test/mocks/controller_mock.h @@ -33,16 +33,17 @@ class ControllerMock : public ControllerInterface { public: MOCK_METHOD0(init, void()); MOCK_METHOD0(update, void()); + MOCK_CONST_METHOD2(com, void(const uint8_t *buffer, size_t size)); MOCK_METHOD0(cacheEncoders, void()); MOCK_METHOD1(setState, void(OpInterface *state)); - MOCK_METHOD0(getState, OpInterface*()); + MOCK_CONST_METHOD0(getState, OpInterface*()); MOCK_METHOD1(setMachineType, void(Machine_t machineType)); - MOCK_METHOD0(getMachineType, Machine_t()); - MOCK_METHOD0(getBeltShift, BeltShift_t()); - MOCK_METHOD0(getCarriage, Carriage_t()); - MOCK_METHOD0(getDirection, Direction_t()); - MOCK_METHOD0(getHallActive, Direction_t()); - MOCK_METHOD0(getPosition, uint8_t()); + MOCK_CONST_METHOD0(getMachineType, Machine_t()); + MOCK_CONST_METHOD0(getBeltShift, BeltShift_t()); + MOCK_CONST_METHOD0(getCarriage, Carriage_t()); + MOCK_CONST_METHOD0(getDirection, Direction_t()); + MOCK_CONST_METHOD0(getHallActive, Direction_t()); + MOCK_CONST_METHOD0(getPosition, uint8_t()); }; ControllerMock *controllerMockInstance(); diff --git a/test/mocks/opKnit_mock.cpp b/test/mocks/opKnit_mock.cpp index d2f1788cf..1952bbebd 100644 --- a/test/mocks/opKnit_mock.cpp +++ b/test/mocks/opKnit_mock.cpp @@ -76,11 +76,6 @@ Err_t OpKnit::startKnitting(uint8_t startNeedle, pattern_start, continuousReportingEnabled); } -void OpKnit::encodePosition() { - assert(gOpKnitMock != nullptr); - gOpKnitMock->encodePosition(); -} - void OpKnit::knit() { assert(gOpKnitMock != nullptr); gOpKnitMock->knit(); diff --git a/test/mocks/opKnit_mock.h b/test/mocks/opKnit_mock.h index fc702187a..a0a188939 100644 --- a/test/mocks/opKnit_mock.h +++ b/test/mocks/opKnit_mock.h @@ -41,7 +41,6 @@ class OpKnitMock : public OpKnitInterface { MOCK_METHOD4(startKnitting, Err_t(uint8_t startNeedle, uint8_t stopNeedle, uint8_t *pattern_start, bool continuousReportingEnabled)); - MOCK_METHOD0(encodePosition, void()); MOCK_METHOD0(knit, void()); MOCK_METHOD1(getStartOffset, uint8_t(const Direction_t direction)); MOCK_METHOD1(setNextLine, bool(uint8_t lineNumber)); diff --git a/test/test_OpKnit.cpp b/test/test_OpKnit.cpp index 3d79c88ae..335f05142 100644 --- a/test/test_OpKnit.cpp +++ b/test/test_OpKnit.cpp @@ -308,19 +308,6 @@ TEST_F(OpKnitTest, test_com) { ASSERT_TRUE(Mock::VerifyAndClear(comMock)); } -TEST_F(OpKnitTest, test_encodePosition) { - opKnit->m_sOldPosition = controller->getPosition(); - EXPECT_CALL(*comMock, send_indState).Times(0); - opKnit->encodePosition(); - - opKnit->m_sOldPosition += 1; - EXPECT_CALL(*comMock, send_indState).Times(1); - opKnit->encodePosition(); - - // test expectations without destroying instance - ASSERT_TRUE(Mock::VerifyAndClear(comMock)); -} - TEST_F(OpKnitTest, test_cacheISR) { expected_cacheISR(); diff --git a/test/test_OpTest.cpp b/test/test_OpTest.cpp index 7567b2a84..bbae33e79 100644 --- a/test/test_OpTest.cpp +++ b/test/test_OpTest.cpp @@ -287,21 +287,18 @@ TEST_F(OpTestTest, test_autoRead) { // nothing has happened yet EXPECT_CALL(*arduinoMock, millis).WillOnce(Return(TEST_LOOP_DELAY - 1)); - EXPECT_CALL(*opKnitMock, encodePosition); expect_readEOLsensors(false); expect_readEncoders(false); opTest->update(); // m_timerEventOdd = false EXPECT_CALL(*arduinoMock, millis).WillOnce(Return(TEST_LOOP_DELAY)); - EXPECT_CALL(*opKnitMock, encodePosition); expect_readEOLsensors(false); expect_readEncoders(false); opTest->update(); // m_timerEventOdd = true EXPECT_CALL(*arduinoMock, millis).WillOnce(Return(2 * TEST_LOOP_DELAY)); - EXPECT_CALL(*opKnitMock, encodePosition); expect_send(false); expect_readEOLsensors(true); expect_readEncoders(true); @@ -310,7 +307,6 @@ TEST_F(OpTestTest, test_autoRead) { // after `stopCmd()` opTest->stopCmd(); EXPECT_CALL(*arduinoMock, millis).Times(0); - EXPECT_CALL(*opKnitMock, encodePosition).Times(0); expect_readEOLsensors(false); expect_readEncoders(false); opTest->update(); @@ -326,14 +322,12 @@ TEST_F(OpTestTest, test_autoTest) { // nothing has happened yet EXPECT_CALL(*arduinoMock, millis).WillOnce(Return(TEST_LOOP_DELAY - 1)); - EXPECT_CALL(*opKnitMock, encodePosition); EXPECT_CALL(*arduinoMock, digitalWrite(LED_PIN_A, HIGH)).Times(0); EXPECT_CALL(*arduinoMock, digitalWrite(LED_PIN_B, HIGH)).Times(0); opTest->update(); // m_timerEventOdd = false EXPECT_CALL(*arduinoMock, millis).WillOnce(Return(TEST_LOOP_DELAY)); - EXPECT_CALL(*opKnitMock, encodePosition); expect_send(true); EXPECT_CALL(*arduinoMock, digitalWrite(LED_PIN_A, HIGH)); EXPECT_CALL(*arduinoMock, digitalWrite(LED_PIN_B, HIGH)); @@ -341,7 +335,6 @@ TEST_F(OpTestTest, test_autoTest) { // m_timerEventOdd = true EXPECT_CALL(*arduinoMock, millis).WillOnce(Return(2 * TEST_LOOP_DELAY)); - EXPECT_CALL(*opKnitMock, encodePosition); expect_send(true); EXPECT_CALL(*arduinoMock, digitalWrite(LED_PIN_A, LOW)); EXPECT_CALL(*arduinoMock, digitalWrite(LED_PIN_B, LOW)); @@ -350,7 +343,6 @@ TEST_F(OpTestTest, test_autoTest) { // after `stopCmd()` opTest->stopCmd(); EXPECT_CALL(*arduinoMock, millis).Times(0); - EXPECT_CALL(*opKnitMock, encodePosition).Times(0); EXPECT_CALL(*arduinoMock, digitalWrite(LED_PIN_A, _)).Times(0); EXPECT_CALL(*arduinoMock, digitalWrite(LED_PIN_B, _)).Times(0); opTest->update(); diff --git a/test/test_com.cpp b/test/test_com.cpp index 943413046..2bd3c4439 100644 --- a/test/test_com.cpp +++ b/test/test_com.cpp @@ -96,6 +96,8 @@ class ComTest : public ::testing::Test { void expected_write_onPacketReceived(uint8_t *buffer, size_t size, bool once) { + EXPECT_CALL(*controllerMock, com(buffer, size)); + com->onPacketReceived(buffer, size); expect_send(once); opTest->com(buffer, size); } diff --git a/test/test_controller.cpp b/test/test_controller.cpp index 96919ab7b..358311996 100644 --- a/test/test_controller.cpp +++ b/test/test_controller.cpp @@ -285,6 +285,15 @@ TEST_F(ControllerTest, test_update_test) { ASSERT_TRUE(Mock::VerifyAndClear(opTestMock)); } +TEST_F(ControllerTest, test_com) { + const uint8_t buffer[] = {0xFF}; + EXPECT_CALL(*opIdleMock, com); + controller->com(buffer, 1); + + // test expectations without destroying instance + ASSERT_TRUE(Mock::VerifyAndClear(opIdleMock)); +} + TEST_F(ControllerTest, test_error_state) { controller->setState(opErrorMock); expected_update_idle();