summaryrefslogtreecommitdiff
path: root/keyboards/handwired/lagrange
diff options
context:
space:
mode:
authorDimitris Papavasiliou <dpapavas@protonmail.ch>2021-04-20 20:17:39 +0300
committerGitHub <noreply@github.com>2021-04-20 10:17:39 -0700
commitbd07120d3303ee38498e01bbef9052d4d50f77f5 (patch)
tree48ac3549b24f43172ceea06ffdd18fc14c30ed62 /keyboards/handwired/lagrange
parent4ff16fe73e0850beeca446d53902527c2cbabe8a (diff)
[Keyboard] Fix and improve SPI transport in the Lagrange (#12606)
Co-authored-by: Dimitris Papavasiliou <dpapavas@gmail.com>
Diffstat (limited to 'keyboards/handwired/lagrange')
-rw-r--r--keyboards/handwired/lagrange/transport.c81
1 files changed, 57 insertions, 24 deletions
diff --git a/keyboards/handwired/lagrange/transport.c b/keyboards/handwired/lagrange/transport.c
index 2a567f24b9..8f6973925f 100644
--- a/keyboards/handwired/lagrange/transport.c
+++ b/keyboards/handwired/lagrange/transport.c
@@ -18,6 +18,7 @@
#include "quantum.h"
#include "split_util.h"
+#include "transport.h"
#include "timer.h"
#include "lagrange.h"
@@ -32,15 +33,16 @@ uint8_t transceive(uint8_t b) {
return SPDR;
}
-/* The SPI bus, doens't have any form of protocol built in, so when
+/* The SPI bus, doesn't have any form of protocol built in, so when
* the other side isn't present, any old noise on the line will appear
* as matrix data. To avoid interpreting data as keystrokes, we do a
* simple n-way (8-way here) handshake before each scan, where each
* side sends a prearranged sequence of bytes. */
-void shake_hands(bool master) {
+bool shake_hands(bool master) {
const uint8_t m = master ? 0xf8 : 0;
const uint8_t a = 0xa8 ^ m, b = 0x50 ^ m;
+ bool synchronized = true;
uint8_t i;
@@ -48,7 +50,7 @@ void shake_hands(bool master) {
i = SPDR;
do {
- /* Cylcling the SS pin on each attempt is necessary, as it
+ /* Cycling the SS pin on each attempt is necessary, as it
* resets the AVR's SPI core and guarantees proper
* alignment. */
@@ -58,6 +60,7 @@ void shake_hands(bool master) {
for (i = 0 ; i < 8 ; i += 1) {
if (transceive(a + i) != b + i) {
+ synchronized = false;
break;
}
}
@@ -66,9 +69,11 @@ void shake_hands(bool master) {
writePinHigh(SPI_SS_PIN);
}
} while (i < 8);
+
+ return synchronized;
}
-bool transport_master(matrix_row_t matrix[]) {
+bool transport_master(matrix_row_t master_matrix[], matrix_row_t slave_matrix[]) {
const struct led_context context = {
host_keyboard_led_state(),
layer_state
@@ -76,32 +81,58 @@ bool transport_master(matrix_row_t matrix[]) {
uint8_t i;
- /* Shake hands and then receive the matrix from the other side,
- * while transmitting LED and layer states. */
+ /* We shake hands both before and after transmitting the matrix.
+ * Doing it before transmitting is necessary to ensure
+ * synchronization: Due to the master-slave nature of the SPI bus,
+ * the master calls the shots. If we just go ahead and start
+ * clocking bits, the slave side might be otherwise engaged at
+ * that moment, so we'll initially read zeros, or garbage. Then
+ * when the slave gets around to transmitting its matrix, we'll
+ * misinterpret the keys it sends, leading to spurious
+ * keypresses. */
- shake_hands(true);
+ /* The handshake forces the master to wait for the slave to be
+ * ready to start transmitting. */
+
+ do {
+ shake_hands(true);
- spi_start(SPI_SS_PIN, 0, 0, 4);
+ /* Receive the matrix from the other side, while transmitting
+ * LED and layer states. */
- for (i = 0 ; i < sizeof(matrix_row_t[MATRIX_ROWS / 2]) ; i += 1) {
- spi_status_t x;
+ spi_start(SPI_SS_PIN, 0, 0, 4);
- x = spi_write(i < sizeof(struct led_context) ?
- ((uint8_t *)&context)[i] : 0);
+ for (i = 0 ; i < sizeof(matrix_row_t[MATRIX_ROWS / 2]) ; i += 1) {
+ spi_status_t x;
+
+ x = spi_write(i < sizeof(struct led_context) ?
+ ((uint8_t *)&context)[i] : 0);
+
+ if (x == SPI_STATUS_TIMEOUT) {
+ return false;
+ }
- if (x == SPI_STATUS_TIMEOUT) {
- return false;
+ ((uint8_t *)slave_matrix)[i] = (uint8_t)x;
}
- ((uint8_t *)matrix)[i] = (uint8_t)x;
- }
+ spi_stop();
+
+ /* In case of errors during the transmission, e.g. if the
+ * cable was disconnected and since there is no inherent
+ * error-checking protocol, we would simply interpret noise as
+ * data. */
+
+ /* To avoid this, both sides shake hands after transmitting.
+ * If synchronization was lost during transmission, the (first)
+ * handshake will fail. In that case we go around and
+ * re-transmit. */
- spi_stop();
+ } while (!shake_hands(true));
return true;
}
-void transport_slave(matrix_row_t matrix[]) {
+void transport_slave(matrix_row_t master_matrix[], matrix_row_t slave_matrix[]) {
static struct led_context context;
struct led_context new_context;
@@ -113,15 +144,17 @@ void transport_slave(matrix_row_t matrix[]) {
cli();
shake_hands(false);
- for (i = 0 ; i < sizeof(matrix_row_t[MATRIX_ROWS / 2]) ; i += 1) {
- uint8_t b;
+ do {
+ for (i = 0 ; i < sizeof(matrix_row_t[MATRIX_ROWS / 2]) ; i += 1) {
+ uint8_t b;
- b = transceive(((uint8_t *)matrix)[i]);
+ b = transceive(((uint8_t *)slave_matrix)[i]);
- if (i < sizeof(struct led_context)) {
- ((uint8_t *)&new_context)[i] = b;
+ if (i < sizeof(struct led_context)) {
+ ((uint8_t *)&new_context)[i] = b;
+ }
}
- }
+ } while (!shake_hands(false));
sei();