From 69da90e25f4796dd32ccf12c86fe19760c6cde15 Mon Sep 17 00:00:00 2001 From: Jonathan Perret Date: Fri, 9 Aug 2024 02:01:43 +0200 Subject: [PATCH] knitter: wait for both selectors to clear the limit We now maintain a "current line direction" that allows us to know whether the carriage is on the expected side of the working area for the line to be considered complete. Then we get rid of m_workedOnLine that is no longer useful. We also use the current line direction when determining which needle to program, making this more reliable than depending on the instantaneous carriage direction. Had to fix a few tests that used a mix of Knit and Garter carriage. --- src/ayab/knitter.cpp | 57 +++++++++++++++------ src/ayab/knitter.h | 3 +- test/test_knitter.cpp | 115 ++++++++++++++++++++++++++++++++++++++---- 3 files changed, 150 insertions(+), 25 deletions(-) diff --git a/src/ayab/knitter.cpp b/src/ayab/knitter.cpp index f7a939d5e..5ff5afc78 100644 --- a/src/ayab/knitter.cpp +++ b/src/ayab/knitter.cpp @@ -73,7 +73,6 @@ void Knitter::init() { m_lastLineFlag = false; m_sOldPosition = 0U; m_firstRun = true; - m_workedOnLine = false; m_lastHall = Direction_t::NoDirection; m_position = 0U; m_hallActive = Direction_t::NoDirection; @@ -167,6 +166,7 @@ Err_t Knitter::startKnitting(uint8_t startNeedle, // reset variables to start conditions m_currentLineNumber = UINT8_MAX; // because counter will // be incremented before request + m_currentLineDirection = Direction::NoDirection; m_lineRequested = false; m_lastLineFlag = false; @@ -241,6 +241,9 @@ void Knitter::knit() { m_firstRun = false; GlobalBeeper::finishedLine(); ++m_currentLineNumber; + // first line direction is based on which Hall sensor was passed + m_currentLineDirection = + m_lastHall == Direction::Left ? Direction::Right : Direction::Left; reqLine(m_currentLineNumber); } @@ -249,8 +252,8 @@ void Knitter::knit() { bool state = digitalRead(DBG_BTN_PIN); if (m_prevState && !state && !m_lineRequested) { - ++m_currentLineNumber; - reqLine(m_currentLineNumber); + ++m_currentLineNumber; + reqLine(m_currentLineNumber); } m_prevState = state; #else @@ -282,18 +285,12 @@ void Knitter::knit() { // write Pixel state to the appropriate needle GlobalSolenoids::setSolenoid(m_solenoidToSet, pixelValue); - if ((m_pixelToSet >= m_startNeedle) && (m_pixelToSet <= m_stopNeedle)) { - m_workedOnLine = true; - } - - if (((m_pixelToSet < m_startNeedle - END_OF_LINE_OFFSET_L[static_cast(m_machineType)]) || - (m_pixelToSet > m_stopNeedle + END_OF_LINE_OFFSET_R[static_cast(m_machineType)])) && - m_workedOnLine) { - // outside of the active needles and - // already worked on the current line -> finished the line - m_workedOnLine = false; - + if (isLineFinished()) { if (!m_lineRequested && !m_lastLineFlag) { + // flip line direction + m_currentLineDirection = m_currentLineDirection == Direction::Left + ? Direction::Right + : Direction::Left; // request new line from host ++m_currentLineNumber; reqLine(m_currentLineNumber); @@ -388,7 +385,7 @@ bool Knitter::calculatePixelAndSolenoid() { uint8_t startOffset = 0; uint8_t laceOffset = 0; - switch (m_direction) { + switch (m_currentLineDirection) { // calculate the solenoid and pixel to be set // implemented according to machine manual // magic numbers from machine manual @@ -447,6 +444,36 @@ bool Knitter::calculatePixelAndSolenoid() { return true; } +/*! + * \brief Assess whether the current line is done being knitted, based on the + * position of the carriage. + * \return `true` if the line is finished, `false` * otherwise. + */ +bool Knitter::isLineFinished() { + // Compute the position of both "needle selectors" as they both need to clear + // the last needle before the carriage can safely turn around. + // Using `int` here as the results may be negative. + int leftSelectorPosition = m_position - getStartOffset(Direction::Left); + int rightSelectorPosition = m_position - getStartOffset(Direction::Right); + + // The limits we are testing against are the working needles plus a safety + // margin + int leftLimit = + m_startNeedle - END_OF_LINE_OFFSET_L[static_cast(m_machineType)]; + int rightLimit = + m_stopNeedle + END_OF_LINE_OFFSET_R[static_cast(m_machineType)]; + + // If going left, both selectors must have cleared the left limit before we + // consider the row done; if going right, both selectors must have cleared the + // right limit. + return ((m_currentLineDirection == Direction::Left && + leftSelectorPosition < leftLimit && + rightSelectorPosition < leftLimit) || + (m_currentLineDirection == Direction::Right && + leftSelectorPosition > rightLimit && + rightSelectorPosition > rightLimit)); +} + /*! * \brief Finish knitting procedure. */ diff --git a/src/ayab/knitter.h b/src/ayab/knitter.h index 70c04792c..a84350e15 100644 --- a/src/ayab/knitter.h +++ b/src/ayab/knitter.h @@ -110,6 +110,7 @@ class Knitter : public KnitterInterface { private: void reqLine(uint8_t lineNumber); bool calculatePixelAndSolenoid(); + bool isLineFinished(); void stopKnitting() const; // job parameters @@ -128,11 +129,11 @@ class Knitter : public KnitterInterface { bool m_lineRequested; uint8_t m_currentLineNumber; + Direction m_currentLineDirection; bool m_lastLineFlag; uint8_t m_sOldPosition; bool m_firstRun; - bool m_workedOnLine; Direction_t m_lastHall; #ifdef DBG_NOMACHINE bool m_prevState; diff --git a/test/test_knitter.cpp b/test/test_knitter.cpp index 392f12e33..77e0a8ce7 100644 --- a/test/test_knitter.cpp +++ b/test/test_knitter.cpp @@ -35,6 +35,9 @@ using ::testing::_; using ::testing::AtLeast; +using ::testing::AtMost; +using ::testing::AnyNumber; +using ::testing::Assign; using ::testing::Mock; using ::testing::Return; using ::testing::TypedEq; @@ -397,7 +400,7 @@ TEST_F(KnitterTest, test_knit_Kh910) { expected_dispatch_knit(false); // no useful position calculated by `calculatePixelAndSolenoid()` - expected_isr(100, Direction_t::NoDirection, Direction_t::Right, BeltShift::Shifted, Carriage_t::Knit); + expected_isr(0, Direction_t::Right, Direction_t::Right, BeltShift::Shifted, Carriage_t::Knit); EXPECT_CALL(*solenoidsMock, setSolenoid).Times(0); expect_indState(); expected_dispatch_knit(false); @@ -421,6 +424,89 @@ TEST_F(KnitterTest, test_knit_Kh910) { ASSERT_TRUE(Mock::VerifyAndClear(comMock)); } +TEST_F(KnitterTest, test_knit_should_not_request_new_line_if_carriage_goes_back_one_needle_inside_working_area) { + get_to_ready(Machine_t::Kh910); + + // not interested in beeps for this test + EXPECT_CALL(*beeperMock, finishedLine).Times(AnyNumber()); + + // 0-only pattern for the first row + uint8_t pattern[25] = { }; + + // all solenoids should be set to 0 only for the first row + EXPECT_CALL(*solenoidsMock, setSolenoid(_, 0)).Times(AnyNumber()); + + const uint8_t startNeedle = 0; + const uint8_t stopNeedle = 100; + const Carriage carriageType = Carriage::Garter; + knitter->startKnitting(startNeedle, stopNeedle, pattern, /* continuous_reporting */ false); + + // first knit: should request first line + EXPECT_CALL(*comMock, send_reqLine); + knitter->knit(); + + // confirm reception of first line + knitter->setNextLine(0); + + // offset between working needle and m_position, for K carriage moving rightwards + const int startOffset = + START_OFFSET[(int)MachineType::Kh910] + // yes, Direction::Left is for a carriage moving rightwards + [(int)Direction::Left][(int)carriageType]; + + int currentPosition; + + // working on a needle near the end of the work area + expected_isr(currentPosition = 90 + startOffset, Direction::Right, Direction::NoDirection, BeltShift::Regular, carriageType); + knitter->knit(); + + // going back left one needle + expected_isr(--currentPosition, + Direction::Left, + Direction::NoDirection, BeltShift::Regular, carriageType); + // should not request a new line! + EXPECT_CALL(*comMock, send_reqLine).Times(0); + knitter->knit(); + + // going rightwards to the last needle in work (stop_needle) + expected_isr(currentPosition = stopNeedle + startOffset, Direction::Right, Direction::NoDirection, BeltShift::Regular, carriageType); + // should not request a new line yet + EXPECT_CALL(*comMock, send_reqLine).Times(0); + knitter->knit(); + + // going rightwards to just outside the work area + expected_isr(++currentPosition, Direction::Right, Direction::NoDirection, BeltShift::Regular, carriageType); + // should not request a new line (because of END_OF_LINE_OFFSET_R) + EXPECT_CALL(*comMock, send_reqLine).Times(0); + knitter->knit(); + + // move rightwards until the line is requested + bool lineWasRequested = false; + EXPECT_CALL(*comMock, send_reqLine(1, _)).Times(AtMost(1)).WillOnce(Assign(&lineWasRequested, true)); + while (!lineWasRequested) { + // safety valve if the line is never requested + ASSERT_LT(currentPosition, 200); + // going rightwards one needle at a time + expected_isr(++currentPosition, Direction::Right, Direction::NoDirection, BeltShift::Regular, carriageType); + // at some point it should request a new line + knitter->knit(); + } + + // the line should have been requested at a point where the + // leftwards needle checker is outside of the work area + const int leftwardsOffset = + START_OFFSET[(int)MachineType::Kh910] + // yes, Direction::Right is for a carriage moving leftwards + [(int)Direction::Right][(int)carriageType]; + ASSERT_GT(currentPosition - leftwardsOffset, stopNeedle); + + // test expectations without destroying instance + ASSERT_TRUE(Mock::VerifyAndClear(solenoidsMock)); + ASSERT_TRUE(Mock::VerifyAndClear(encodersMock)); + ASSERT_TRUE(Mock::VerifyAndClear(beeperMock)); + ASSERT_TRUE(Mock::VerifyAndClear(comMock)); +} + TEST_F(KnitterTest, test_knit_Kh270) { get_to_ready(Machine_t::Kh270); @@ -447,7 +533,7 @@ TEST_F(KnitterTest, test_knit_Kh270) { expected_dispatch_knit(false); // no useful position calculated by `calculatePixelAndSolenoid()` - expected_isr(60, Direction_t::NoDirection, Direction_t::Right, BeltShift::Shifted, Carriage_t::Knit); + expected_isr(0, Direction_t::Right, Direction_t::Right, BeltShift::Shifted, Carriage_t::Knit); EXPECT_CALL(*solenoidsMock, setSolenoid).Times(0); expect_indState(); expected_dispatch_knit(false); @@ -498,13 +584,19 @@ TEST_F(KnitterTest, test_knit_lastLine) { // Run one knit inside the working needles. EXPECT_CALL(*solenoidsMock, setSolenoid); - expected_isr(knitter->getStartOffset(Direction_t::Left) + 20); + expected_isr(knitter->getStartOffset(Direction_t::Left) + 20, + Direction::Right, Direction::Left, BeltShift::Regular, + Carriage::Knit); // `m_workedOnLine` is set to true expected_dispatch_knit(false); // Position has changed since last call to operate function // `m_pixelToSet` is above `m_stopNeedle` + END_OF_LINE_OFFSET_R - expected_isr(NUM_NEEDLES[static_cast(Machine_t::Kh910)] + END_OF_LINE_OFFSET_R[static_cast(Machine_t::Kh910)] + 1 + knitter->getStartOffset(Direction_t::Left)); + expected_isr( + NUM_NEEDLES[static_cast(Machine_t::Kh910)] + + END_OF_LINE_OFFSET_R[static_cast(Machine_t::Kh910)] + 1 + + knitter->getStartOffset(Direction_t::Left), + Direction::Right, Direction::Left, BeltShift::Regular, Carriage::Knit); // `m_lastLineFlag` is `true` knitter->setLastLine(); @@ -530,9 +622,8 @@ TEST_F(KnitterTest, test_knit_lastLine_and_no_req) { uint8_t wanted_pixel = knitter->m_stopNeedle + END_OF_LINE_OFFSET_R[static_cast(Machine_t::Kh910)] + 1; knitter->m_firstRun = false; - knitter->m_direction = Direction_t::Left; - knitter->m_position = wanted_pixel + knitter->getStartOffset(Direction_t::Right); - knitter->m_workedOnLine = true; + knitter->m_currentLineDirection = Direction_t::Right; + knitter->m_position = wanted_pixel + knitter->getStartOffset(Direction_t::Left); knitter->m_lineRequested = false; knitter->m_lastLineFlag = true; @@ -574,13 +665,19 @@ TEST_F(KnitterTest, test_knit_new_line) { // Run one knit inside the working needles. EXPECT_CALL(*solenoidsMock, setSolenoid); - expected_isr(knitter->getStartOffset(Direction_t::Left) + 20); + expected_isr(knitter->getStartOffset(Direction_t::Left) + 20, + Direction::Right, Direction::Left, BeltShift::Regular, + Carriage::Knit); // `m_workedOnLine` is set to true expected_dispatch_knit(false); // Position has changed since last call to operate function // `m_pixelToSet` is above `m_stopNeedle` + END_OF_LINE_OFFSET_R - expected_isr(NUM_NEEDLES[static_cast(Machine_t::Kh910)] + END_OF_LINE_OFFSET_R[static_cast(Machine_t::Kh910)] + 1 + knitter->getStartOffset(Direction_t::Left)); + expected_isr( + NUM_NEEDLES[static_cast(Machine_t::Kh910)] + + END_OF_LINE_OFFSET_R[static_cast(Machine_t::Kh910)] + 1 + + knitter->getStartOffset(Direction_t::Left), + Direction::Right, Direction::Left, BeltShift::Regular, Carriage::Knit); // set `m_lineRequested` to `false` EXPECT_CALL(*beeperMock, finishedLine);