Simplify KH270 carriage detection and fix offsets

Author: jonathanperret

src/ayab/encoders.cpp

@@ -193,27 +193,30 @@ void Encoders::encA_rising() {
       return;
     }
 
-    // For KH-270, if the carriage is already set, ignore the rest.
-    if (Machine_t::Kh270 == m_machineType && Carriage_t::Knit == 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;
+    // The KH270 carriage also has two magnets, but:
+    // - the sensors and magnets are set up such that the carriage center
+    //   is in front of the first/last needle when the carriage's outermost
+    //   magnet is in front of the sensor;
+    // - we will always see the outermost magnet last and reset the position
+    //   to a correct value, so it's not a problem if we detect the innermost
+    //   first, even though that gives us a wrong position for a few needles
+    //   (since we're not selecting anything at that point this has no impact)
+    // So there's no need to special-case position detection for the KH270 carriage.
+
+    // KH270 has no belt shift
+    if (m_machineType != Machine_t::Kh270) {
+      // 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;
+      }
     }
 
     uint8_t start_position = END_LEFT_PLUS_OFFSET[static_cast<uint8_t>(m_machineType)];
 
-    if (m_machineType == Machine_t::Kh270) {
-      m_carriage = Carriage_t::Knit;
-
-      // Assume the rightmost magnet was detected
-      start_position = start_position + MAGNET_DISTANCE_270;
-    } else if (m_carriage == Carriage_t::Lace &&
+    if (m_carriage == Carriage_t::Lace &&
                detected_carriage == Carriage_t::Knit &&
                m_position > start_position) {
       m_carriage = Carriage_t::Garter;
@@ -281,29 +284,32 @@ void Encoders::encA_falling() {
       return;
     }
 
-    // For KH-270, if the carriage is already set, ignore the rest.
-    if (Machine_t::Kh270 == m_machineType && Carriage_t::Knit == 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
-      m_beltShift = digitalRead(ENC_PIN_C) != 0 ? BeltShift::Regular : BeltShift::Shifted;
-      m_passedRight = true;
-
-      // Shift doesn't need to be swapped for the g-carriage in this direction.
+    // The KH270 carriage also has two magnets, but:
+    // - the sensors and magnets are set up such that the carriage center
+    //   is in front of the first/last needle when the carriage's outermost
+    //   magnet is in front of the sensor;
+    // - we will always see the outermost magnet last and reset the position
+    //   to a correct value, so it's not a problem if we detect the innermost
+    //   first, even though that gives us a wrong position for a few needles
+    //   (since we're not selecting anything at that point this has no impact)
+    // So there's no need to special-case position detection for the KH270 carriage.
+
+    // KH270 has no belt shift
+    if (m_machineType != Machine_t::Kh270) {
+      // 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
+        m_beltShift = digitalRead(ENC_PIN_C) != 0 ? BeltShift::Regular : BeltShift::Shifted;
+        m_passedRight = true;
+
+        // Shift doesn't need to be swapped for the g-carriage in this direction.
+      }
     }
 
     uint8_t start_position = END_RIGHT_MINUS_OFFSET[static_cast<uint8_t>(m_machineType)];
 
-    if (m_machineType == Machine_t::Kh270) {
-      m_carriage = Carriage_t::Knit;
-
-      // Assume the leftmost magnet was detected
-      start_position = start_position; // FIXME
-    } else if (m_carriage == Carriage_t::Lace &&
+    if (m_carriage == Carriage_t::Lace &&
                detected_carriage == Carriage_t::Knit &&
                m_position < start_position) {
       m_carriage = Carriage_t::Garter;

src/ayab/encoders.h

@@ -66,16 +66,16 @@ constexpr uint8_t END_OF_LINE_OFFSET_L[NUM_MACHINES] = {12U, 12U, 6U};
 constexpr uint8_t END_OF_LINE_OFFSET_R[NUM_MACHINES] = {12U, 12U, 6U};
 
 constexpr uint8_t END_LEFT[NUM_MACHINES] = {0U, 0U, 0U};
-constexpr uint8_t END_RIGHT[NUM_MACHINES] = {255U, 255U, 140U};
-constexpr uint8_t END_OFFSET[NUM_MACHINES] = {28U, 28U, 5U};
+constexpr uint8_t END_RIGHT[NUM_MACHINES] = {255U, 255U, 167U};
+constexpr uint8_t END_OFFSET[NUM_MACHINES] = {28U, 28U, 28U};
 
 // The following two arrays are created by combining, respectively,
 // the arrays END_LEFT and END_RIGHT with END_OFFSET
-constexpr uint8_t END_LEFT_PLUS_OFFSET[NUM_MACHINES] = {28U, 28U, 5U};
-constexpr uint8_t END_RIGHT_MINUS_OFFSET[NUM_MACHINES] = {227U, 227U, 135U};
+constexpr uint8_t END_LEFT_PLUS_OFFSET[NUM_MACHINES] = {28U, 28U, 28U};
+constexpr uint8_t END_RIGHT_MINUS_OFFSET[NUM_MACHINES] = {227U, 227U, 139U};
 
-constexpr uint8_t ALL_MAGNETS_CLEARED_LEFT[NUM_MACHINES] = {56U, 56U, 10U};
-constexpr uint8_t ALL_MAGNETS_CLEARED_RIGHT[NUM_MACHINES] = {199U, 199U, 130U};
+constexpr uint8_t ALL_MAGNETS_CLEARED_LEFT[NUM_MACHINES] = {56U, 56U, 38U};
+constexpr uint8_t ALL_MAGNETS_CLEARED_RIGHT[NUM_MACHINES] = {199U, 199U, 129U};
 
 // The garter slop is needed to determine whether or not we have a garter carriage.
 // If we didn't have it, we'd decide which carriage we had when the first magnet passed the sensor.
@@ -102,8 +102,8 @@ constexpr uint8_t START_OFFSET[NUM_MACHINES][NUM_DIRECTIONS][NUM_CARRIAGES] = {
     // KH270
     {
         // K
-        {28U, 0U, 0U}, // Left
-        {16U, 0U, 0U} // Right
+        {28U + 6U, 0U, 0U}, // Left
+        {28U - 6U, 0U, 0U} // Right
     }};
 
 // Should be calibrated to each device
@@ -117,8 +117,6 @@ constexpr uint16_t FILTER_R_MAX[NUM_MACHINES] = {1023U, 600U, 600U};
 
 constexpr uint16_t SOLENOIDS_BITMASK = 0xFFFFU;
 
-constexpr uint8_t MAGNET_DISTANCE_270 = 12U;
-
 /*!
  * \brief Encoder interface.
  *