From c402b78d95f5c6c64f5c489f8c7378402a12f25a Mon Sep 17 00:00:00 2001 From: Jonathan Perret Date: Mon, 14 Oct 2024 00:38:23 +0200 Subject: [PATCH] encoders: fix carriage detection on the right Mostly by copying the logic from encA_rising into encA_falling, and special-casing the KH910 with its inverted K-only reading. Also, if we have detected a Garter carriage, we already decided that it's too hard to try to reset the position when it crosses the turn marks. If we punt on resetting the belt shift as well, we save some logic and fix the few weird spots where it turns while its outer magnet is right on the turn mark. --- src/ayab/encoders.cpp | 96 +++++++++++++++++++++++++++--------------- src/ayab/encoders.h | 6 ++- test/test_encoders.cpp | 24 +++++++---- 3 files changed, 82 insertions(+), 44 deletions(-) diff --git a/src/ayab/encoders.cpp b/src/ayab/encoders.cpp index 478a24b8..0bc1c5e1 100644 --- a/src/ayab/encoders.cpp +++ b/src/ayab/encoders.cpp @@ -150,31 +150,19 @@ void Encoders::encA_rising() { (hallValue > FILTER_L_MAX[static_cast(m_machineType)])) { m_hallActive = Direction_t::Left; + // The garter carriage has a second set of magnets that are going to + // pass the sensor and will reset state incorrectly if allowed to + // continue. + if (Carriage_t::Garter == m_carriage) { + return; + } + // Only set the belt shift the first time a magnet passes the turn mark. // Headed to the right. if (!m_passedLeft && Direction_t::Right == m_direction) { // Belt shift signal only decided in front of hall sensor m_beltShift = digitalRead(ENC_PIN_C) != 0 ? BeltShift::Shifted : BeltShift::Regular; m_passedLeft = true; - - if (Carriage_t::Garter == m_carriage) { - // This has to be the first magnet and the belt shift needs to be swapped - // But only for the G-carriage - if (m_position < 30) { - if (BeltShift::Regular == m_beltShift) { - m_beltShift = BeltShift::Shifted; - } else { - m_beltShift = BeltShift::Regular; - } - } - } - } - - // The garter carriage has a second set of magnets that are going to - // pass the sensor and will reset state incorrectly if allowed to - // continue. - if (Carriage_t::Garter == m_carriage) { - return; } // If the carriage is already set, ignore the rest. @@ -200,7 +188,9 @@ void Encoders::encA_rising() { } } else if (m_carriage == Carriage_t::NoCarriage) { m_carriage = detected_carriage; - } else if (m_carriage != detected_carriage && m_position > start_position) { + } else if (m_carriage == Carriage::Lace && + detected_carriage == Carriage::Knit && + m_position > start_position) { m_carriage = Carriage_t::Garter; // We swap the belt shift for the g-carriage because the point of work for @@ -250,17 +240,18 @@ void Encoders::encA_falling() { // In front of Right Hall Sensor? uint16_t hallValue = analogRead(EOL_PIN_R); - - // Avoid 'comparison of unsigned expression < 0 is always false' - // by being explicit about that behaviour being expected. - bool hallValueSmall = false; - - hallValueSmall = (hallValue < FILTER_R_MIN[static_cast(m_machineType)]); - - if (hallValueSmall || hallValue > FILTER_R_MAX[static_cast(m_machineType)]) { + if ((hallValue < FILTER_R_MIN[static_cast(m_machineType)]) || + (hallValue > FILTER_R_MAX[static_cast(m_machineType)])) { m_hallActive = Direction_t::Right; - // Only set the belt shift when the first magnet passes the turn mark. + // The garter carriage has a second set of magnets that are going to + // pass the sensor and will reset state incorrectly if allowed to + // continue. + if (Carriage_t::Garter == m_carriage) { + return; + } + + // Only set the belt shift the first time a magnet passes the turn mark. // Headed to the left. if (!m_passedRight && Direction_t::Left == m_direction) { // Belt shift signal only decided in front of hall sensor @@ -270,18 +261,53 @@ void Encoders::encA_falling() { // Shift doens't need to be swapped for the g-carriage in this direction. } - // The garter carriage has extra magnets that are going to - // pass the sensor and will reset state incorrectly if allowed to - // continue. - if (m_carriage == Carriage_t::Garter) { + // If the carriage is already set, ignore the rest. + if ((Carriage_t::Knit == m_carriage) && (Machine_t::Kh270 == m_machineType)) { return; } - if (hallValueSmall) { + Carriage detected_carriage = Carriage_t::NoCarriage; + uint8_t start_position = END_RIGHT_MINUS_OFFSET[static_cast(m_machineType)]; + + if (m_machineType == MachineType::Kh910) { + // Due to an error in wiring on the shield, the sensor only triggers for the K carriage, + // and with a low voltage so we can't use the same logic as for other machines. + detected_carriage = Carriage_t::Knit; + } else { + if (hallValue >= FILTER_R_MIN[static_cast(m_machineType)]) { + detected_carriage = Carriage_t::Knit; + } else { + detected_carriage = Carriage_t::Lace; + } + } + + if (m_machineType == Machine_t::Kh270) { m_carriage = Carriage_t::Knit; + + // The first magnet on the carriage looks like Lace, the second looks like Knit + if (detected_carriage == Carriage_t::Knit) { + start_position = start_position + MAGNET_DISTANCE_270; + } + } else if (m_carriage == Carriage_t::NoCarriage) { + m_carriage = detected_carriage; + } else if (m_carriage == Carriage::Lace && + detected_carriage == Carriage::Knit && + m_position < start_position) { + m_carriage = Carriage_t::Garter; + + // Belt shift and start position were set when the first magnet passed + // the sensor and we assumed we were working with a standard carriage. + + // Because we detected the leftmost magnet on the G-carriage first, + // for consistency with the left side where we detect the rightmost magnet + // first, we need to adjust the carriage position. + m_position += GARTER_L_MAGNET_SPACING; + return; + } else { + m_carriage = detected_carriage; } // Known position of the carriage -> overwrite position - m_position = END_RIGHT_MINUS_OFFSET[static_cast(m_machineType)]; + m_position = start_position; } } diff --git a/src/ayab/encoders.h b/src/ayab/encoders.h index be6c8588..61ddbc64 100644 --- a/src/ayab/encoders.h +++ b/src/ayab/encoders.h @@ -81,6 +81,10 @@ constexpr uint8_t ALL_MAGNETS_CLEARED_RIGHT[NUM_MACHINES] = {199U, 199U, 130U}; // If we didn't have it, we'd decide which carriage we had when the first magnet passed the sensor. // For the garter carriage we need to see both magnets. constexpr uint8_t GARTER_SLOP = 2U; +// Spacing between a garter carriage's outer (L-carriage-like) magnets. +// For consistency between a garter carriage starting on the left or the right, +// we need to adjust the position by this distance when starting from the right. +constexpr uint8_t GARTER_L_MAGNET_SPACING = 24U; constexpr uint8_t START_OFFSET[NUM_MACHINES][NUM_DIRECTIONS][NUM_CARRIAGES] = { // KH910 @@ -108,7 +112,7 @@ constexpr uint8_t START_OFFSET[NUM_MACHINES][NUM_DIRECTIONS][NUM_CARRIAGES] = { // KH910 KH930 KH270 constexpr uint16_t FILTER_L_MIN[NUM_MACHINES] = { 200U, 200U, 200U}; constexpr uint16_t FILTER_L_MAX[NUM_MACHINES] = { 600U, 600U, 600U}; -constexpr uint16_t FILTER_R_MIN[NUM_MACHINES] = { 200U, 0U, 0U}; +constexpr uint16_t FILTER_R_MIN[NUM_MACHINES] = { 200U, 200U, 200U}; constexpr uint16_t FILTER_R_MAX[NUM_MACHINES] = {1023U, 600U, 600U}; constexpr uint16_t SOLENOIDS_BITMASK = 0xFFFFU; diff --git a/test/test_encoders.cpp b/test/test_encoders.cpp index ff84f17a..76f1b81c 100644 --- a/test/test_encoders.cpp +++ b/test/test_encoders.cpp @@ -131,30 +131,34 @@ TEST_F(EncodersTest, test_encA_rising_in_front_G_carriage) { // Create a rising edge EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_A)).WillOnce(Return(true)); // Enter rising function, direction is right - EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_B)).WillOnce(Return(true)).WillOnce(Return(false)); + EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_B)).WillOnce(Return(true)); // In front of Left Hall Sensor EXPECT_CALL(*arduinoMock, analogRead(EOL_PIN_L)) - .WillOnce(Return(FILTER_L_MAX[static_cast(encoders->getMachineType())] + 1)); + .WillOnce(Return(FILTER_L_MIN[static_cast(encoders->getMachineType())] - 1)); // BeltShift is regular EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_C)).WillOnce(Return(true)); encoders->encA_interrupt(); - ASSERT_EQ(encoders->getCarriage(), Carriage_t::Knit); + ASSERT_EQ(encoders->getCarriage(), Carriage_t::Lace); // Create a falling edge EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_A)).WillOnce(Return(false)); + // Enter falling function, direction is right + EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_B)).WillOnce(Return(false)); + // Not in front of Right Hall sensor EXPECT_CALL(*arduinoMock, analogRead(EOL_PIN_R)) - .WillOnce(Return(FILTER_R_MAX[static_cast(encoders->getMachineType())] + 1)); - EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_C)); + .WillOnce(Return(FILTER_R_MAX[static_cast(encoders->getMachineType())] - 1)); + encoders->encA_interrupt(); + // Create a rising edge EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_A)).WillOnce(Return(true)); // Enter rising function, direction is right EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_B)).WillOnce(Return(true)); // In front of Left Hall Sensor EXPECT_CALL(*arduinoMock, analogRead(EOL_PIN_L)) - .WillOnce(Return(FILTER_L_MIN[static_cast(encoders->getMachineType())] - 1)); + .WillOnce(Return(FILTER_R_MAX[static_cast(encoders->getMachineType())] + 1)); encoders->encA_interrupt(); @@ -185,6 +189,8 @@ TEST_F(EncodersTest, test_encA_falling_not_in_front) { } TEST_F(EncodersTest, test_encA_falling_in_front) { + encoders->init(Machine_t::Kh930); + ASSERT_TRUE(encoders->getMachineType() == Machine_t::Kh930); // Create a falling edge EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_A)) .WillOnce(Return(true)) @@ -209,17 +215,19 @@ TEST_F(EncodersTest, test_encA_falling_in_front) { ASSERT_EQ(encoders->getDirection(), Direction_t::Right); ASSERT_EQ(encoders->getHallActive(), Direction_t::Right); ASSERT_EQ(encoders->getPosition(), 227); - ASSERT_EQ(encoders->getCarriage(), Carriage_t::NoCarriage); + ASSERT_EQ(encoders->getCarriage(), Carriage_t::Knit); } // requires FILTER_R_MIN != 0 TEST_F(EncodersTest, test_encA_falling_set_K_carriage_KH910) { + encoders->init(Machine_t::Kh910); ASSERT_TRUE(encoders->getMachineType() == Machine_t::Kh910); // Create a rising edge EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_A)).WillOnce(Return(true)); EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_B)); - EXPECT_CALL(*arduinoMock, analogRead(EOL_PIN_L)); + EXPECT_CALL(*arduinoMock, analogRead(EOL_PIN_L)) + .WillOnce(Return(FILTER_R_MIN[static_cast(encoders->getMachineType())] + 1)); encoders->encA_interrupt(); // falling edge