From 1038bbef40b795a06d0866edacc9d24b210fb146 Mon Sep 17 00:00:00 2001 From: MakotoKurauchi Date: Thu, 28 Jun 2018 22:55:15 +0900 Subject: [PATCH] Helix serial bugfix (#3255) * add helix serial debug code * serial debug macro move from config.h to serial.h * helix serial.c debugging... * refine debug macros * add some comments * add SELECT_SERIAL_SPEED * add comments * debugging sync_send/sync_recv * add very high speed * fix sync_send/sync_recv * fix com. start and switch send/recv * debug mode off * remove debug codes --- keyboards/helix/serial.c | 149 +++++++++++++++++++++++++++------------ 1 file changed, 102 insertions(+), 47 deletions(-) diff --git a/keyboards/helix/serial.c b/keyboards/helix/serial.c index 51ab67fb3d..182f222196 100644 --- a/keyboards/helix/serial.c +++ b/keyboards/helix/serial.c @@ -14,9 +14,45 @@ #ifdef USE_SERIAL -// Serial pulse period in microseconds. Its probably a bad idea to lower this -// value. -#define SERIAL_DELAY 30 +#define _delay_sub_us(x) __builtin_avr_delay_cycles(x) + +// Serial pulse period in microseconds. +#define SELECT_SERIAL_SPEED 1 +#if SELECT_SERIAL_SPEED == 0 + // Very High speed + #define SERIAL_DELAY 4 // micro sec + #define READ_WRITE_START_ADJUST 30 // cycles + #define READ_WRITE_WIDTH_ADJUST 10 // cycles +#elif SELECT_SERIAL_SPEED == 1 + // High speed + #define SERIAL_DELAY 6 // micro sec + #define READ_WRITE_START_ADJUST 23 // cycles + #define READ_WRITE_WIDTH_ADJUST 10 // cycles +#elif SELECT_SERIAL_SPEED == 2 + // Middle speed + #define SERIAL_DELAY 12 // micro sec + #define READ_WRITE_START_ADJUST 25 // cycles + #define READ_WRITE_WIDTH_ADJUST 10 // cycles +#elif SELECT_SERIAL_SPEED == 3 + // Low speed + #define SERIAL_DELAY 24 // micro sec + #define READ_WRITE_START_ADJUST 25 // cycles + #define READ_WRITE_WIDTH_ADJUST 10 // cycles +#elif SELECT_SERIAL_SPEED == 4 + // Very Low speed + #define SERIAL_DELAY 50 // micro sec + #define READ_WRITE_START_ADJUST 25 // cycles + #define READ_WRITE_WIDTH_ADJUST 10 // cycles +#else +#error Illegal Serial Speed +#endif + + +#define SERIAL_DELAY_HALF1 (SERIAL_DELAY/2) +#define SERIAL_DELAY_HALF2 (SERIAL_DELAY - SERIAL_DELAY/2) + +#define SLAVE_INT_WIDTH 1 +#define SLAVE_INT_RESPONSE_TIME SERIAL_DELAY uint8_t volatile serial_slave_buffer[SERIAL_SLAVE_BUFFER_LENGTH] = {0}; uint8_t volatile serial_master_buffer[SERIAL_MASTER_BUFFER_LENGTH] = {0}; @@ -28,9 +64,17 @@ inline static void serial_delay(void) { _delay_us(SERIAL_DELAY); } -void serial_delay_short(void) { - _delay_us(SERIAL_DELAY-1); + +inline static +void serial_delay_half1(void) { + _delay_us(SERIAL_DELAY_HALF1); +} + +inline static +void serial_delay_half2(void) { + _delay_us(SERIAL_DELAY_HALF2); } + inline static void serial_output(void) { SERIAL_PIN_DDR |= SERIAL_PIN_MASK; @@ -38,7 +82,7 @@ void serial_output(void) { // make the serial pin an input with pull-up resistor inline static -void serial_input(void) { +void serial_input_with_pullup(void) { SERIAL_PIN_DDR &= ~SERIAL_PIN_MASK; SERIAL_PIN_PORT |= SERIAL_PIN_MASK; } @@ -64,7 +108,7 @@ void serial_master_init(void) { } void serial_slave_init(void) { - serial_input(); + serial_input_with_pullup(); #ifndef USE_SERIAL_PD2 // Enable INT0 @@ -79,25 +123,21 @@ void serial_slave_init(void) { #endif } -// Used by the master to synchronize timing with the slave. +// Used by the sender to synchronize timing with the reciver. static void sync_recv(void) { - serial_input(); + for (int i = 0; i < SERIAL_DELAY*5 && serial_read_pin(); i++ ) { + } // This shouldn't hang if the slave disconnects because the // serial line will float to high if the slave does disconnect. while (!serial_read_pin()); - //serial_delay(); - _delay_us(SERIAL_DELAY-5); } -// Used by the slave to send a synchronization signal to the master. +// Used by the reciver to send a synchronization signal to the sender. static void sync_send(void) { - serial_output(); - serial_low(); serial_delay(); - serial_high(); } @@ -105,66 +145,70 @@ void sync_send(void) { static uint8_t serial_read_byte(void) { uint8_t byte = 0; - serial_input(); + _delay_sub_us(READ_WRITE_START_ADJUST); for ( uint8_t i = 0; i < 8; ++i) { + serial_delay_half1(); // read the middle of pulses byte = (byte << 1) | serial_read_pin(); - serial_delay(); - _delay_us(1); + _delay_sub_us(READ_WRITE_WIDTH_ADJUST); + serial_delay_half2(); } - return byte; } // Sends a byte with MSB ordering static void serial_write_byte(uint8_t data) { - uint8_t b = 8; - serial_output(); - while( b-- ) { - if(data & (1 << b)) { + uint8_t b = 1<<7; + while( b ) { + if(data & b) { serial_high(); } else { serial_low(); } + b >>= 1; serial_delay(); } + serial_low(); // sync_send() / senc_recv() need raise edge } // interrupt handle to be used by the slave device ISR(SERIAL_PIN_INTERRUPT) { - sync_send(); + serial_output(); + // slave send phase uint8_t checksum = 0; for (int i = 0; i < SERIAL_SLAVE_BUFFER_LENGTH; ++i) { - serial_write_byte(serial_slave_buffer[i]); sync_send(); + serial_write_byte(serial_slave_buffer[i]); checksum += serial_slave_buffer[i]; } - serial_write_byte(checksum); sync_send(); + serial_write_byte(checksum); - // wait for the sync to finish sending - serial_delay(); - - // read the middle of pulses - _delay_us(SERIAL_DELAY/2); + // slave switch to input + sync_send(); //0 + serial_delay_half1(); //1 + serial_low(); //2 + serial_input_with_pullup(); //2 + serial_delay_half1(); //3 + // slave recive phase uint8_t checksum_computed = 0; for (int i = 0; i < SERIAL_MASTER_BUFFER_LENGTH; ++i) { + sync_recv(); serial_master_buffer[i] = serial_read_byte(); - sync_send(); checksum_computed += serial_master_buffer[i]; } + sync_recv(); uint8_t checksum_received = serial_read_byte(); - sync_send(); - - serial_input(); // end transaction if ( checksum_computed != checksum_received ) { status |= SLAVE_DATA_CORRUPT; } else { status &= ~SLAVE_DATA_CORRUPT; } + + sync_recv(); //weit master output to high } inline @@ -178,6 +222,7 @@ bool serial_slave_DATA_CORRUPT(void) { // Returns: // 0 => no error // 1 => slave did not respond +// 2 => checksum error int serial_update_buffers(void) { // this code is very time dependent, so we need to disable interrupts cli(); @@ -185,51 +230,61 @@ int serial_update_buffers(void) { // signal to the slave that we want to start a transaction serial_output(); serial_low(); - _delay_us(1); + _delay_us(SLAVE_INT_WIDTH); // wait for the slaves response - serial_input(); - serial_high(); - _delay_us(SERIAL_DELAY); + serial_input_with_pullup(); + _delay_us(SLAVE_INT_RESPONSE_TIME); // check if the slave is present if (serial_read_pin()) { // slave failed to pull the line low, assume not present + serial_output(); + serial_high(); sei(); return 1; } + // master recive phase // if the slave is present syncronize with it - sync_recv(); uint8_t checksum_computed = 0; // receive data from the slave for (int i = 0; i < SERIAL_SLAVE_BUFFER_LENGTH; ++i) { - serial_slave_buffer[i] = serial_read_byte(); sync_recv(); + serial_slave_buffer[i] = serial_read_byte(); checksum_computed += serial_slave_buffer[i]; } - uint8_t checksum_received = serial_read_byte(); sync_recv(); + uint8_t checksum_received = serial_read_byte(); if (checksum_computed != checksum_received) { + serial_output(); + serial_high(); sei(); return 2; } + // master switch to output + sync_recv(); //0 + serial_delay(); //1 + serial_low(); //3 + serial_output(); // 3 + serial_delay_half1(); //4 + + // master send phase uint8_t checksum = 0; - // send data to the slave + for (int i = 0; i < SERIAL_MASTER_BUFFER_LENGTH; ++i) { + sync_send(); serial_write_byte(serial_master_buffer[i]); - sync_recv(); checksum += serial_master_buffer[i]; } + sync_send(); serial_write_byte(checksum); - sync_recv(); // always, release the line when not in use - serial_output(); - serial_high(); + sync_send(); sei(); return 0;