diff --git a/src/ayab/encoders.cpp b/src/ayab/encoders.cpp index c5c63043..b4ae19e3 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_t::Lace && + detected_carriage == Carriage_t::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 @@ -248,19 +238,25 @@ void Encoders::encA_falling() { } } + // No attempt to support KH270 on the right for now as I can't test it + if (m_machineType == Machine_t::Kh270) { + return; + } + // 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 +266,39 @@ 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) { - return; + 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 (hallValueSmall) { - m_carriage = Carriage_t::Knit; + if (m_carriage == Carriage_t::Lace && + detected_carriage == Carriage_t::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 28c7256d..776cb698 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