Fix clunking sound from solenoids

The "clunking" sound that is occasionnally heard when using AYAB
is caused by all solenoid armatures being released simultaneously
when all solenoids are turned off.

This does not happen with Brother electronics because they default
to keeping solenoids on.

This commit changes AYAB's behavior to match that behavior of the
Brother electronics. Note that this required abandoning the third-party
library used to drive the I/O expanders (Adafruit_MCP23008) since
part of its non-skippable initialization code was forcing all outputs
to off. It turns out the amount of code needed to directly drive the
MCP23008 expanders using the Arduino Wire library is only a dozen
lines of code any way.

Fixes AllYarnsAreBeautiful#15.

.gitmodules

@@ -1,6 +1,3 @@
 [submodule "libraries/PacketSerial"]
 	path = lib/PacketSerial
-	url = https://github.com/bakercp/PacketSerial.git
-[submodule "libraries/Adafruit_MCP23008"]
-	url = https://github.com/adafruit/Adafruit-MCP23008-library.git
-	path = lib/Adafruit_MCP23008
+	url = https://github.com/bakercp/PacketSerial.git

src/ayab/solenoids.cpp

@@ -1,4 +1,4 @@
-/*!
+ /*!
  * \file solenoids.cpp
  * \brief Class containing methods that control the needles
  *    via solenoids connected to IO expanders on the device.
@@ -25,18 +25,29 @@
 
 #include "solenoids.h"
 
+// Minimal required constants from the MCP23008 datasheet:
+// - fixed I2C address bits
+#define MCP23008_ADDRESS 0x20
+// - used register addresses
+#define MCP23008_IODIR 0x00
+#define MCP23008_GPIO 0x09
+
 /*!
  * \brief Initialize I2C connection for solenoids.
  */
 void Solenoids::init() {
-  mcp_0.begin(I2Caddr_sol1_8);
-  mcp_1.begin(I2Caddr_sol9_16);
+  Wire.begin();
 
-  for (uint8_t i = 0; i < SOLENOID_BUFFER_SIZE / 2; i++) {
-    mcp_0.pinMode(i, OUTPUT);
-    mcp_1.pinMode(i, OUTPUT);
-  }
-  solenoidState = 0x0000U;
+  // Default to all solenoids ON
+  // This mimicks the behavior of Brother electronics and is done
+  // to avoid a "clunking" sound that occurs if solenoids are turned
+  // OFF after having been ON while the carriage was moving.
+  solenoidState = 0xffffU;
+  writeGPIO(solenoidState);
+
+  // Configure all MCP23008 pins as outputs
+  writeRegister(I2Caddr_sol1_8, MCP23008_IODIR, 0x00);
+  writeRegister(I2Caddr_sol9_16, MCP23008_IODIR, 0x00);
 }
 
 /*!
@@ -59,7 +70,7 @@ void Solenoids::setSolenoid(uint8_t solenoid, bool state) {
   }
   if (oldState != solenoidState) {
 #ifndef AYAB_TESTS
-    write(solenoidState);
+    writeGPIO(solenoidState);
 #endif
   }
 }
@@ -74,7 +85,7 @@ void Solenoids::setSolenoids(uint16_t state) {
   if (state != solenoidState) {
     solenoidState = state;
 #ifndef AYAB_TESTS
-    write(state);
+    writeGPIO(state);
 #endif
   }
 }
@@ -93,9 +104,23 @@ void Solenoids::setSolenoids(uint16_t state) {
  * one bit per solenoid.
  */
 // GCOVR_EXCL_START
-void Solenoids::write(uint16_t newState) {
-  (void)newState;
-  mcp_0.writeGPIO(lowByte(newState));
-  mcp_1.writeGPIO(highByte(newState));
+void Solenoids::writeGPIO(uint16_t newState) {
+  writeRegister(I2Caddr_sol1_8, MCP23008_GPIO, lowByte(newState));
+  writeRegister(I2Caddr_sol9_16, MCP23008_GPIO, highByte(newState));
+}
+
+/*!
+ * Write to an MCP23008 register via I2C
+ *
+ * \param i2caddr Address of the I/O expander, only the 3 lowest bits are
+ * used, mapping to the A0..A2 address pins on MCP23008
+ * \param reg Register address (see MCP23008 datasheet)
+ * \param value Value to set the register to
+ */
+void Solenoids::writeRegister(uint8_t i2caddr, uint8_t reg, uint8_t value) {
+  Wire.beginTransmission(MCP23008_ADDRESS | (i2caddr & 7));
+  Wire.write(reg);
+  Wire.write(value);
+  Wire.endTransmission();
 }
-// GCOVR_EXCL_STOP
+// GCOVR_EXCL_STOP

src/ayab/solenoids.h

@@ -27,7 +27,6 @@
 #include "board.h"
 #include "encoders.h"
 #include <Arduino.h>
-#include <Adafruit_MCP23008.h>
 #include <Wire.h>
 
 // Different machines have a different number of solenoids.
@@ -82,10 +81,9 @@ class Solenoids : public SolenoidsInterface {
 
 private:
   uint16_t solenoidState = 0x0000U;
-  void write(uint16_t state);
+  void writeGPIO(uint16_t state);
 
-  Adafruit_MCP23008 mcp_0 = Adafruit_MCP23008();
-  Adafruit_MCP23008 mcp_1 = Adafruit_MCP23008();
+  void writeRegister(uint8_t i2caddr, uint8_t reg, uint8_t value);
 };
 
 #endif // SOLENOIDS_H_

static_analysis.sh

@@ -1,2 +1,2 @@
 #!/bin/bash
-clang-tidy src/ayab/$1.cpp --fix -- -isystem /usr/share/arduino/hardware/arduino/cores/arduino/ -isystem /usr/lib/avr/include/ -isystem /usr/share/arduino/hardware/arduino/variants/standard -isystem libraries/Adafruit_MCP23008/ -isystem /usr/share/arduino/libraries/Wire/ -isystem libraries/PacketSerial/src/ -DCLANG_TIDY "${@:2}"
+clang-tidy src/ayab/$1.cpp --fix -- -isystem /usr/share/arduino/hardware/arduino/cores/arduino/ -isystem /usr/lib/avr/include/ -isystem /usr/share/arduino/hardware/arduino/variants/standard -isystem /usr/share/arduino/libraries/Wire/ -isystem libraries/PacketSerial/src/ -DCLANG_TIDY "${@:2}"

test/CMakeLists.txt

@@ -31,9 +31,6 @@ set(COMMON_INCLUDES
     ${SOURCE_DIRECTORY}
     ${LIBRARY_DIRECTORY}/PacketSerial/src
     )
-set(EXTERNAL_LIB_INCLUDES
-    ${LIBRARY_DIRECTORY}/Adafruit_MCP23008
-    )
 set(COMMON_SOURCES
     ${PROJECT_SOURCE_DIR}/test_boards.cpp
 
@@ -85,16 +82,10 @@ set(COMMON_LINKER_FLAGS
     ${CMAKE_THREAD_LIBS_INIT}
     -lgcov
     )
-set(HARD_I2C_LIB
-    ${LIBRARY_DIRECTORY}/Adafruit_MCP23008/Adafruit_MCP23008.cpp
-    )
 function(add_board board)
     set(processor # uno
         __AVR_ATmega168__
         )
-    set(I2C_LIB
-        ${HARD_I2C_LIB}
-        )
     add_executable(${PROJECT_NAME}_${board}
         ${COMMON_SOURCES}
         # External libraries

test/test_solenoids.cpp

@@ -50,7 +50,7 @@ TEST_F(SolenoidsTest, test_construct) {
 
 TEST_F(SolenoidsTest, test_init) {
   solenoids->init();
-  ASSERT_TRUE(solenoids->solenoidState == 0U);
+  ASSERT_TRUE(solenoids->solenoidState == 0xffffU);
 }
 
 TEST_F(SolenoidsTest, test_setSolenoid1) {