diff --git a/avr_code/adc.c b/avr_code/adc.c index 6d19a0f..9736873 100644 --- a/avr_code/adc.c +++ b/avr_code/adc.c @@ -1,6 +1,15 @@ #include "adc.h" -int8_t adc_Initialize() +#define ADC_CONVERSION_TIMEOUT 1000U + +// The ADC needs a 50-200 kHz clock. At F_CPU = 8 MHz that is a /64 prescaler +// (125 kHz); the old /2 ran it at 4 MHz, far out of spec. +#define ADC_PRESCALER_64 ((1 << ADPS2) | (1 << ADPS1)) + +// The 1.1 V bandgap reference needs time to settle after the mux is switched. +#define ADC_SETTLE_US 200 + +int8_t adc_Initialize(void) { //REFS VAL_0x01; ADLAR disabled; MUX adc0; ADMUX = 0x40; @@ -8,18 +17,17 @@ int8_t adc_Initialize() //ACME disabled; ADTS VAL_0x00; ADCSRB = 0x00; - //ADEN enabled; ADSC disabled; ADATE disabled; ADIF disabled; ADIE disabled; ADPS VAL_0x01; - ADCSRA = 0x81; + ADCSRA = (1 << ADEN) | ADC_PRESCALER_64; return 0; } -void adc_Disable() +void adc_Disable(void) { ADCSRA &= ~(1 << ADEN); } -void adc_Enable() +void adc_Enable(void) { ADCSRA |= (1 << ADEN); } @@ -42,25 +50,37 @@ void adc_StartConversion(uint8_t channel) ADMUX &= ~0x0f; ADMUX |= channel; } + _delay_us(ADC_SETTLE_US); ADCSRA |= (1 << ADSC); } -bool adc_IsConversionDone() +bool adc_IsConversionDone(void) { return ((ADCSRA & (1 << ADIF))); } uint16_t adc_GetConversionResult(void) { - return (ADCL | ADCH << 8); + // ADC reads ADCL then ADCH in the right order. Reading the two volatile + // registers in one expression leaves the order unspecified, and taking ADCH + // first breaks the data-register lock and corrupts the result. + return ADC; } uint16_t adc_GetConversion(uint8_t channel) { - - adc_StartConversion(channel); - while (!adc_IsConversionDone()); + + // A conversion is 13 ADC clocks (~104 us at 125 kHz); bail out rather than + // hang if the ADC is disabled or its clock is gated off. + uint16_t attempts = 0; + while (!adc_IsConversionDone()) { + if (++attempts > ADC_CONVERSION_TIMEOUT) { + return 0; + } + _delay_us(10); + } + uint16_t res = adc_GetConversionResult(); ADCSRA |= (1 << ADIF); return res; diff --git a/avr_code/adc.h b/avr_code/adc.h index 8e662bd..78a5fea 100644 --- a/avr_code/adc.h +++ b/avr_code/adc.h @@ -7,20 +7,22 @@ #ifndef ADC_H #define ADC_H +#include "defines.h" #include -#include #include +#include +#include #ifdef __cplusplus extern "C" { #endif -int8_t adc_Initialize(); -void adc_Enable(); -void adc_Disable(); +int8_t adc_Initialize(void); +void adc_Enable(void); +void adc_Disable(void); void adc_StartConversion(uint8_t channel); -bool adc_IsConversionDone(); +bool adc_IsConversionDone(void); uint16_t adc_GetConversionResult(void); uint16_t adc_GetConversion(uint8_t channel); diff --git a/avr_code/defines.c b/avr_code/defines.c index 904874a..1ddbd75 100644 --- a/avr_code/defines.c +++ b/avr_code/defines.c @@ -1,6 +1,6 @@ #include "defines.h" - unsigned char DATA_BUFFER_65[64]; + unsigned char DATA_BUFFER_65[65]; uint8_t DATA_BUFFER_7[7]; // uint8_t DATA_BUFFER_254[255]; unsigned char DATA_BUFFER_20[20]; diff --git a/avr_code/defines.h b/avr_code/defines.h index c828e12..cb7c361 100644 --- a/avr_code/defines.h +++ b/avr_code/defines.h @@ -17,7 +17,9 @@ extern "C" { #include #include #include -#define F_CPU 8000000UL // 16 MHz clock speed +#ifndef F_CPU +#define F_CPU 8000000UL // 8 MHz clock speed; prefer -DF_CPU=8000000UL in the build flags +#endif #define BAUD 38400 #define F_SCL 200000UL @@ -27,14 +29,14 @@ extern "C" { #define MIN(a,b) (((a)<(b))?(a):(b)) #define MAX(a,b) (((a)>(b))?(a):(b)) -extern unsigned char DATA_BUFFER_65[64]; +extern unsigned char DATA_BUFFER_65[65]; extern uint8_t DATA_BUFFER_7[7]; //extern uint8_t DATA_BUFFER_254[255]; extern unsigned char DATA_BUFFER_20[20]; typedef struct { uint8_t payload_len; - char payload[255]; + char payload[48]; uint8_t success; } ndef_message; extern ndef_message NDEF_MSG; @@ -47,8 +49,8 @@ extern trimmed_string_struct TRIMMED_STRING; typedef struct { uint8_t name_len; - char name_str[128]; - char diameter_str[128]; + char name_str[16]; + char diameter_str[16]; uint8_t diameter_len; uint8_t hashed; } identifier_results; @@ -103,10 +105,6 @@ typedef enum // Goes into flags -#define WHILE_BREAK(counter, attempts) \ - counter+=1; \ - if ((counter+1) > attempts) { break;}; - #ifdef __cplusplus } #endif diff --git a/avr_code/i2c.c b/avr_code/i2c.c index 8fe4511..4b41c7c 100644 --- a/avr_code/i2c.c +++ b/avr_code/i2c.c @@ -1,7 +1,20 @@ #include "i2c.h" #include -void i2c_init() +// Every one of these loops used to spin forever. The peripheral rail is cut +// before sleeping, so a device that is slow or absent on wake would otherwise +// hang the firmware with no watchdog reset armed. +static bool i2c_wait_twint(void) +{ + for (uint16_t attempts = 0; attempts < I2C_TIMEOUT_LOOPS; attempts++) { + if (TWCR & (1 << TWINT)) { + return true; + } + } + return false; +} + +void i2c_init(void) { // Set SCL and SDA as inputs (automatically done by TWI hardware) TWSR = 0; // Prescaler = 1 @@ -13,13 +26,13 @@ void i2c_init() uint8_t i2c_start(uint8_t address) { TWCR = (1 << TWSTA) | (1 << TWINT) | (1 << TWEN); // Send START condition - while (!(TWCR & (1 << TWINT))) - ; // Wait for TWINT flag to be set + if (!i2c_wait_twint()) + return 1; TWDR = address; // Load address into data register TWCR = (1 << TWINT) | (1 << TWEN); // Send address - while (!(TWCR & (1 << TWINT))) - ; // Wait for TWINT flag to be set + if (!i2c_wait_twint()) + return 1; uint8_t status = TWSR & 0xF8; if (status != 0x18 && status != 0x40) return 1; @@ -36,9 +49,15 @@ uint8_t write_one_byte(uint8_t device_addr, uint8_t register_addr, uint8_t data) uint8_t write_n_bytes(uint8_t device_addr, uint8_t register_addr, uint8_t* data, uint8_t n_bytes) { I2C_START_WRITE(device_addr); - i2c_write(register_addr); + if (i2c_write(register_addr)) { + i2c_stop(); + return 1; + } for (uint8_t i = 0; i < n_bytes; i++) { - i2c_write(data[i]); + if (i2c_write(data[i])) { + i2c_stop(); + return 1; + } } i2c_stop(); return 0; @@ -106,18 +125,21 @@ read_n_bytes_16bit_addr(uint8_t device_addr, uint16_t register_addr, uint8_t* da } // Stop i2c communication -void i2c_stop() +void i2c_stop(void) { TWCR = (1 << TWSTO) | (1 << TWINT) | (1 << TWEN); // Send STOP condition - while (!(TWCR & (1 << TWSTO))) - ; // Wait for STOP to complete + for (uint16_t attempts = 0; attempts < I2C_TIMEOUT_LOOPS; attempts++) { + if (!(TWCR & (1 << TWSTO))) { + return; // STOP complete + } + } } -uint8_t i2c_read_ack() +uint8_t i2c_read_ack(void) { TWCR = (1 << TWEN) | (1 << TWINT) | (1 << TWEA); - while (!(TWCR & (1 << TWINT))) - ; // Wait for TWINT flag to be set + if (!i2c_wait_twint()) + return 0xFF; return TWDR; } @@ -126,17 +148,17 @@ uint8_t i2c_write(uint8_t data) // Load data into TWDR TWDR = data; TWCR = (1 << TWEN) | (1 << TWINT); - while (!(TWCR & (1 << TWINT))) - ; // Wait for TWINT flag set + if (!i2c_wait_twint()) + return 1; if ((TWSR & 0xF8) != TW_MT_DATA_ACK) return 1; // Check ACK return 0; } -uint8_t i2c_read_nack() +uint8_t i2c_read_nack(void) { TWCR = (1 << TWEN) | (1 << TWINT); - while (!(TWCR & (1 << TWINT))) - ; // Wait for TWINT flag to be set + if (!i2c_wait_twint()) + return 0xFF; return TWDR; } \ No newline at end of file diff --git a/avr_code/i2c.h b/avr_code/i2c.h index dd0697f..46a71a2 100644 --- a/avr_code/i2c.h +++ b/avr_code/i2c.h @@ -2,6 +2,14 @@ #include "uart.h" #include +#ifndef i2c_H +#define i2c_H + +#define TWSR TWSR0 +#define TWDR TWDR0 +#define TWBR TWBR0 +#define TWCR TWCR0 + #define I2C_START_WRITE(device_addr) \ { \ if (i2c_start((device_addr << 1) | 0x00)) { \ @@ -16,15 +24,11 @@ } \ } -#ifndef i2c_H -#define i2c_H +// A byte at F_SCL takes well under 100 us; anything past this means the bus is +// stuck (peripheral unpowered, SDA held low) and we must not spin forever. +#define I2C_TIMEOUT_LOOPS 20000U -#define TWSR TWSR0 -#define TWDR TWDR0 -#define TWBR TWBR0 -#define TWCR TWCR0 - -void i2c_init(); +void i2c_init(void); uint8_t i2c_start(uint8_t address); uint8_t write_one_byte(uint8_t device_addr, uint8_t register_addr, @@ -45,10 +49,9 @@ uint8_t read_one_byte(uint8_t device_addr, uint8_t register_addr, uint8_t read_n_bytes(uint8_t device_addr, uint8_t register_addr, uint8_t *data, uint8_t n_bytes); -void i2c_stop(); -void i2c_scan(); -uint8_t i2c_read_ack(); -uint8_t i2c_read_nack(); +void i2c_stop(void); +uint8_t i2c_read_ack(void); +uint8_t i2c_read_nack(void); uint8_t i2c_write(uint8_t data); #endif \ No newline at end of file diff --git a/avr_code/interrupts.c b/avr_code/interrupts.c index 7ef72d4..bda89bf 100644 --- a/avr_code/interrupts.c +++ b/avr_code/interrupts.c @@ -1,54 +1,70 @@ #include "interrupts.h" -void init_pins() { +void init_pins(void) { - // Set reed switch interrupt pin + // The reed switch (PD3/INT1) switches to ground and the RTC alarm output + // (PD2/INT0) is open-drain, so both need the internal pull-up. Leaving them + // floating makes the inputs self-trigger. SET_PIN_IN(DDRD, DDD3); - SET_PIN_LOW(PORTD, PD3); + SET_PIN_HIGH(PORTD, PD3); + + SET_PIN_IN(DDRD, DDD2); + SET_PIN_HIGH(PORTD, PD2); } -void set_up_reed_interrupt() { - // Falling edge interrupt - EICRA |= (1 << ISC11); - EICRA &= ~(1 << ISC10); +// Both external interrupts are configured low-level triggered (ISCn1:0 = 00). +// Edge detection needs the I/O clock, which SLEEP_MODE_PWR_DOWN stops, so a +// falling-edge INT0/INT1 can never wake the MCU. Only level detection is +// asynchronous. Each handler masks its own interrupt while the source is still +// asserted, so the low level does not retrigger in a loop. - // Enable INT1 interrupt +void set_up_reed_interrupt(void) { + EICRA &= ~((1 << ISC11) | (1 << ISC10)); EIMSK |= (1 << INT1); } -void set_up_minute_interrupt() { - EICRA |= (1 << ISC01); - EICRA &= ~(1 << ISC00); - - // Enable INT1 interrupt +void set_up_minute_interrupt(void) { + EICRA &= ~((1 << ISC01) | (1 << ISC00)); EIMSK |= (1 << INT0); } -void wdt_isr_enable() { +void reed_interrupt_enable(void) { + EIFR = (1 << INTF1); // Drop anything latched while we were masked + EIMSK |= (1 << INT1); +} + +void minute_interrupt_enable(void) { + EIFR = (1 << INTF0); + EIMSK |= (1 << INT0); +} + +void wdt_isr_enable(void) { + uint8_t sreg = SREG; cli(); wdt_reset(); - WDTCSR |= (1 << WDCE) | (1 << WDE); + // WDRF keeps WDE set, which would block the write below, so it must go + // first. The unlock is a single assignment: a read-modify-write does not + // open the 4-cycle change window. + MCUSR &= ~(1 << WDRF); + WDTCSR = (1 << WDCE) | (1 << WDE); - WDTCSR = (1 << WDP2) | (1 << WDP0); // WDP[3:0] = 0b101 (2 seconds) - WDTCSR |= (1 << WDIE); // Enable WDT Interrupt mode - sei(); + // WDP[3:0] = 0b011 -> 0.125 s. Interrupt mode only (WDE clear), so an + // expiry wakes us to clear the debounce instead of resetting the part. + WDTCSR = (1 << WDIE) | (1 << WDP1) | (1 << WDP0); + + SREG = sreg; // Restore, never blanket-sei(): these run inside an ISR } -void wdt_isr_disable() { - +void wdt_isr_disable(void) { + uint8_t sreg = SREG; cli(); + wdt_reset(); - WDTCSR |= (1 << WDCE) | (1 << WDE); - WDTCSR = 0x00; + MCUSR &= ~(1 << WDRF); + WDTCSR = (1 << WDCE) | (1 << WDE); + WDTCSR = 0x00; - sei(); + SREG = sreg; } -// -//void set_debounce_timer_interrupt() { -// TCCR0A = 0; -// TCCR0B = (1 << CS01) | (1 << CS00); -// TIMSK0 = (1 << TOIE0); -// TCNT0 = 0; -//} diff --git a/avr_code/interrupts.h b/avr_code/interrupts.h index a7dfe1a..a70baf3 100644 --- a/avr_code/interrupts.h +++ b/avr_code/interrupts.h @@ -20,12 +20,13 @@ extern "C" { - void init_pins(); - void set_up_reed_interrupt(); -// void set_debounce_timer_interrupt(); - void set_up_minute_interrupt(); - void wdt_isr_disable(); - void wdt_isr_enable(); + void init_pins(void); + void set_up_reed_interrupt(void); + void set_up_minute_interrupt(void); + void reed_interrupt_enable(void); + void minute_interrupt_enable(void); + void wdt_isr_disable(void); + void wdt_isr_enable(void); #ifdef __cplusplus } diff --git a/avr_code/m95128.c b/avr_code/m95128.c index 8f96c8a..e2d980f 100644 --- a/avr_code/m95128.c +++ b/avr_code/m95128.c @@ -35,15 +35,18 @@ void eeprom_write(uint8_t page, unsigned const char* msg, uint8_t msg_len) spi_write(EEPROM_WRDI); spi_eeprom_select(false); - while (1) { + // Poll the write-in-progress bit, but give up rather than spin forever if + // the EEPROM is unpowered or absent. + for (uint16_t attempts = 0; attempts < EEPROM_POLL_TIMEOUT_MS; attempts++) { spi_eeprom_select(true); spi_write(EEPROM_RDSR); read_value = spi_read(); spi_eeprom_select(false); - if (read_value == 0x00) { + if ((read_value & EEPROM_STATUS_WIP) == 0) { return; } - }; + _delay_ms(1); + } } void eeprom_read(uint8_t page, unsigned char* msg, uint8_t msg_len) @@ -63,7 +66,7 @@ void eeprom_read(uint8_t page, unsigned char* msg, uint8_t msg_len) spi_eeprom_select(false); } -void delete_last_page() +void delete_last_page(void) { old_last_page = get_last_page(); if (old_last_page == 0) // If we have nothing, no need to delete anything @@ -75,7 +78,7 @@ void delete_last_page() eeprom_clear_page(old_last_page); } -uint8_t get_last_page() +uint8_t get_last_page(void) { memset(DATA_BUFFER_65, 0, 1); eeprom_read(0, DATA_BUFFER_65, 1); @@ -127,7 +130,7 @@ tx_rx_data_struct eeprom_read_tx_data(uint8_t page) return TX_DATA; } -tx_rx_data_struct read_struct_last_page() +tx_rx_data_struct read_struct_last_page(void) { uint8_t page_num = get_last_page(); return eeprom_read_tx_data(page_num); diff --git a/avr_code/m95128.h b/avr_code/m95128.h index 229a25b..f6cedc8 100644 --- a/avr_code/m95128.h +++ b/avr_code/m95128.h @@ -26,9 +26,12 @@ #define EEPROM_RDLS 0b10000011 // 0x83 #define EEPROM_LID 0b10000010 // 0x82 #define PAGE_SIZE 64 +#define EEPROM_STATUS_WIP 0x01 + +// A page write takes ~5 ms; past this the device is not responding. +#define EEPROM_POLL_TIMEOUT_MS 100U #ifdef __cplusplus extern "C" { -w #endif void @@ -37,16 +40,16 @@ void eeprom_write(uint8_t page, unsigned const char* msg, uint8_t msg_len); void eeprom_read(uint8_t page, unsigned char* msg, uint8_t msg_len); void eeprom_write_tx_data(uint8_t page, tx_rx_data_struct tx_data); void write_page_address(uint8_t page); -uint8_t get_last_page(); +uint8_t get_last_page(void); tx_rx_data_struct eeprom_read_tx_data(uint8_t page); -void delete_last_page(); -uint8_t get_last_page(); +void delete_last_page(void); +uint8_t get_last_page(void); void write_last_page_value(uint8_t page); void eeprom_clear_page(uint8_t page); void write_page_address(uint8_t page); void eeprom_write_tx_data(uint8_t page, tx_rx_data_struct tx_data); void write_struct_to_last_page(tx_rx_data_struct tx_data); -tx_rx_data_struct read_struct_last_page(); +tx_rx_data_struct read_struct_last_page(void); #ifdef __cplusplus } #endif diff --git a/avr_code/main.c b/avr_code/main.c index 8b417c7..f1904af 100644 --- a/avr_code/main.c +++ b/avr_code/main.c @@ -19,80 +19,96 @@ #include #include #include +#include #include -#define WAIT_FOREVER \ - while (1) { \ - _delay_ms(100); \ - }; - #if ITERATING #define SEND_INTERVAL 1 #else #define SEND_INTERVAL 15 #endif + +#define WHEEL_COUNT_SLOTS 15 + +// Erased EEPROM reads back as 0xFF; anything else is a real page count. +#define EEPROM_LAST_PAGE_UNINIT 0xFF + uint16_t self_value; -tx_rx_data_struct CRAP; -uint16_t main_counter; volatile uint8_t is_debouncing = 0; volatile bool increment_minute_index = false; volatile bool increment_wheel_count = false; volatile uint8_t index_wheel_count = 0; -volatile uint16_t total_wheel_counts[15]; +volatile uint16_t total_wheel_counts[WHEEL_COUNT_SLOTS]; RTC_RFM69_STATUS rtc_rfm69_status; ISR(INT0_vect) { - cli(); + // The RTC holds INTB low until its flag registers are read, and this is a + // level-triggered interrupt, so mask it here and let main re-arm it once + // the RTC has released the line. + EIMSK &= ~(1 << INT0); #if DO_UART uart_sendString("\t\t\t\tMINUTE INTERRUPT\n"); #endif increment_minute_index = true; -// sei(); } ISR(INT1_vect) { - cli(); #if ITERATING increment_minute_index = true; #endif - + #if DO_UART uart_sendString("\t\t\t\tREED INTERRUPT\n"); #endif - if (!is_debouncing) { - total_wheel_counts[index_wheel_count]++; + if (index_wheel_count < WHEEL_COUNT_SLOTS) { + total_wheel_counts[index_wheel_count]++; + } is_debouncing = 1; + // Mask INT1 for the debounce window: the magnet holds the reed closed + // (and the pin low) for far longer than one revolution's worth of + // bounce, and a level-triggered interrupt would retrigger continuously. + EIMSK &= ~(1 << INT1); wdt_isr_enable(); } -// sei(); } ISR(WDT_vect) { - cli(); is_debouncing = 0; wdt_isr_disable(); -// sei(); + reed_interrupt_enable(); } -void start_sleeping() { +void start_sleeping(void) { spi_eeprom_select(false); spi_rfm69_select(false); rfid_set_low_power_down(true); rfid_set_i2c_power(false); ldo_set_state(false); _delay_ms(10); - sleep_bod_disable(); + set_sleep_mode(SLEEP_MODE_PWR_DOWN); - sleep_enable(); + + cli(); + // Don't sleep through work that arrived while we were dropping the rails. + // Testing the flag with interrupts off, then sei() immediately before + // sleep_cpu(), is the avr-libc idiom that closes that race -- and + // sleep_bod_disable() is a timed sequence, so it belongs here and not + // before sleep_enable() where it had no effect at all. + if (!increment_minute_index) { + sleep_enable(); + sleep_bod_disable(); + sei(); + sleep_cpu(); + sleep_disable(); + } sei(); - sleep_cpu(); } -uint16_t get_battery_reading() { +uint16_t get_battery_reading(void) { adc_Enable(); adc_GetConversion(14); adc_GetConversion(14); @@ -149,12 +165,17 @@ int main(void) { uart_sendString("Initialized RFM69\n"); #endif - write_last_page_value(0); + // Only initialise the spool pointer when it has never been written -- + // clearing it unconditionally would discard every unsent message across a + // reset. + if (get_last_page() == EEPROM_LAST_PAGE_UNINIT) { + write_last_page_value(0); + } #if DO_UART uart_sendString("Set up last page value for SPI flash\n"); #endif - for (uint8_t c = 0; c < 15; c++) { + for (uint8_t c = 0; c < WHEEL_COUNT_SLOTS; c++) { total_wheel_counts[c] = 0; } @@ -203,27 +224,43 @@ int main(void) { spi_eeprom_select(false); start_sleeping(); - cli(); - if (increment_minute_index) { + // Short critical sections around the shared variables only. The old + // blanket cli() stayed in force through the whole radio/EEPROM + // sequence, so every reed pulse in that multi-second window was lost. + bool minute_elapsed; + ATOMIC_BLOCK(ATOMIC_RESTORESTATE) { + minute_elapsed = increment_minute_index; + increment_minute_index = false; + } + + if (minute_elapsed) { #if DO_UART uart_sendString("In minute index\n"); #endif - increment_minute_index = false; - is_debouncing = 0; - index_wheel_count += 1; + uint8_t slot; + ATOMIC_BLOCK(ATOMIC_RESTORESTATE) { + if (index_wheel_count < WHEEL_COUNT_SLOTS) { + index_wheel_count += 1; + } + slot = index_wheel_count; + } + + // The I2C rail has to be back up before we touch the RTC: sleeping + // dropped both the LDO and the tag's supply. + ldo_set_state(true); + rfid_set_i2c_power(true); + _delay_ms(1); rtc_read_interrupt_register(); rtc_read_status_register(); - rtc_read_interrupt_register(); - rtc_read_status_register(); + // Reading the flags releases INTB, so INT0 can safely be re-armed. + minute_interrupt_enable(); - if (index_wheel_count >= SEND_INTERVAL) { - index_wheel_count = 0; - ldo_set_state(true); + if (slot >= SEND_INTERVAL) { + ATOMIC_BLOCK(ATOMIC_RESTORESTATE) { index_wheel_count = 0; } rfm69_init(); - rfid_set_i2c_power(true); rfid_set_low_power_down(false); _delay_ms(1); IDENTIFIER = get_nugget_data(); @@ -233,11 +270,21 @@ int main(void) { rtc_rfm69_status = set_time_from_rfm69(IDENTIFIER); } + // Snapshot and clear the counters in one critical section so + // a reed pulse landing mid-packet is neither lost nor double + // counted. + uint16_t counts_snapshot[WHEEL_COUNT_SLOTS]; + ATOMIC_BLOCK(ATOMIC_RESTORESTATE) { + for (uint8_t c = 0; c < WHEEL_COUNT_SLOTS; c++) { + counts_snapshot[c] = total_wheel_counts[c]; + total_wheel_counts[c] = 0; + } + } + // Generate wheel counts message reset_txrx_struct(&TX_DATA); - get_battery_reading(); TX_DATA = generate_wheel_counts_message( - IDENTIFIER, rtc_read_time(), get_battery_reading(), total_wheel_counts); + IDENTIFIER, rtc_read_time(), get_battery_reading(), counts_snapshot); #if DO_UART uart_sendString("TX DATA Sent\n"); @@ -253,28 +300,26 @@ int main(void) { write_struct_to_last_page(TX_DATA); } - for (uint8_t c = 0; c < 15; c++) { - total_wheel_counts[c] = 0; - } if ((result == DATA_SEND_SUCCESS) && (get_last_page() > 0)) { reset_txrx_struct(&TX_DATA); TX_DATA = read_struct_last_page(); TX_DATA.flags = MSG_RESENT_COUNTS; _delay_ms(250); result = send_message(TX_DATA); -#if DO_UART - if (result == DATA_NOT_SENT) { - uart_sendString(" SPI not sent\n"); - } -#endif - - send_message(TX_DATA); - delete_last_page(); #if DO_UART uart_sendString("TX DATA From SPI Memory\n"); uart_print_tx_rx_data(TX_DATA); #endif - + // Only drop the spooled page once it is actually + // acknowledged, otherwise a failed retry loses the data. + if (result == DATA_SEND_SUCCESS) { + delete_last_page(); + } +#if DO_UART + else { + uart_sendString(" SPI not sent\n"); + } +#endif } } } diff --git a/avr_code/max31329.c b/avr_code/max31329.c index 63c4605..dc702af 100644 --- a/avr_code/max31329.c +++ b/avr_code/max31329.c @@ -39,7 +39,7 @@ RTC_RFM69_STATUS set_time_from_rfm69(identifier_results id_data) return RTC_RFM69_SET_TIME_FAILED; } -uint8_t rtc_set_per_minute_alarm() +uint8_t rtc_set_per_minute_alarm(void) { DATA_BUFFER_7[0] = 0x80; @@ -61,17 +61,17 @@ void uart_print_rtc_time(time_struct td) uint8_t rtc_read_register(uint8_t addr) { return read_one_byte_no_err_register(I2C_ADDR, addr); } -uint8_t rtc_read_status_register() { return rtc_read_register(0x00); } +uint8_t rtc_read_status_register(void) { return rtc_read_register(0x00); } -uint8_t rtc_read_interrupt_register() { return rtc_read_register(0x01); } +uint8_t rtc_read_interrupt_register(void) { return rtc_read_register(0x01); } -uint8_t rtc_set_alarm_config() { return write_one_byte(I2C_ADDR, 0x04, 0b00001010); } +uint8_t rtc_set_alarm_config(void) { return write_one_byte(I2C_ADDR, 0x04, 0b00001010); } -uint8_t rtc_enable_interrupts() { return write_one_byte(I2C_ADDR, 0x01, 0b00000010); } +uint8_t rtc_enable_interrupts(void) { return write_one_byte(I2C_ADDR, 0x01, 0b00000010); } uint8_t rtc_read_time_array(uint8_t* data) { return read_n_bytes(I2C_ADDR, 0x06, data, 7); } -time_struct rtc_read_time() +time_struct rtc_read_time(void) { rtc_read_time_array(DATA_BUFFER_7); @@ -94,15 +94,16 @@ uint8_t rtc_write_time(time_struct tm) if (i2c_start((I2C_ADDR << 1) | 0x00)) return 1; - i2c_write(0x06); - i2c_write(DEC2BCD(tm.Second)); - i2c_write(DEC2BCD(tm.Minute)); - i2c_write(DEC2BCD(tm.Hour)); - i2c_write(tm.Wday); - i2c_write(DEC2BCD(tm.Day)); - i2c_write(DEC2BCD(tm.Month)); - i2c_write(DEC2BCD(y2kYearToTm(tm.Year))); + uint8_t err = 0; + err |= i2c_write(0x06); + err |= i2c_write(DEC2BCD(tm.Second)); + err |= i2c_write(DEC2BCD(tm.Minute)); + err |= i2c_write(DEC2BCD(tm.Hour)); + err |= i2c_write(tm.Wday); + err |= i2c_write(DEC2BCD(tm.Day)); + err |= i2c_write(DEC2BCD(tm.Month)); + err |= i2c_write(DEC2BCD(y2kYearToTm(tm.Year))); i2c_stop(); - return 0; + return err ? 1 : 0; } diff --git a/avr_code/max31329.h b/avr_code/max31329.h index 5a0cf18..985c8f8 100644 --- a/avr_code/max31329.h +++ b/avr_code/max31329.h @@ -10,7 +10,6 @@ #include "rfm69.h" #include "st25dv.h" #include "uart.h" -#define I2C_ADDR 0x68 #ifndef MAX31329_H #define MAX31329_H @@ -19,20 +18,25 @@ extern "C" { #endif -#define DEC2BCD(n) (n + (6 * (n / 10))) -#define BCD2DEC(n) (n - (6 * (n >> 4))) +#define I2C_ADDR 0x68 -#define tmYearToY2k(Y) ((Y) - 30) // offset is from 2000 -#define y2kYearToTm(Y) ((Y) + 30) +#define DEC2BCD(n) ((n) + (6 * ((n) / 10))) +#define BCD2DEC(n) ((n) - (6 * ((n) >> 4))) -uint8_t rtc_enable_interrupts(); -uint8_t rtc_set_per_minute_alarm(); +// time_struct.Year is years since 2000, which is exactly what the RTC's 2-digit +// year register holds -- no offset. (The old +/-30 round-tripped but stored the +// wrong year in the RTC and overflowed BCD above 2069.) +#define tmYearToY2k(Y) (Y) +#define y2kYearToTm(Y) (Y) + +uint8_t rtc_enable_interrupts(void); +uint8_t rtc_set_per_minute_alarm(void); uint8_t rtc_read_time_array(uint8_t* data); -time_struct rtc_read_time(); +time_struct rtc_read_time(void); uint8_t rtc_write_time(time_struct tm); -uint8_t rtc_set_alarm_config(); -uint8_t rtc_read_status_register(); -uint8_t rtc_read_interrupt_register(); +uint8_t rtc_set_alarm_config(void); +uint8_t rtc_read_status_register(void); +uint8_t rtc_read_interrupt_register(void); uint8_t rtc_read_register(uint8_t addr); void uart_print_rtc_time(time_struct td); RTC_RFM69_STATUS set_time_from_rfm69(identifier_results id_data); diff --git a/avr_code/ndef.c b/avr_code/ndef.c index 384ebc1..47e9b38 100644 --- a/avr_code/ndef.c +++ b/avr_code/ndef.c @@ -1,10 +1,23 @@ #include "ndef.h" -ndef_message readNDEFText(unsigned char *buf) { - int addr = 0; - NDEF_MSG.success = 0; +// Everything read here comes off an NFC tag that anyone can write, so every +// length taken from the buffer is bounds-checked before it is used. +#define NDEF_NEED(n) \ + { \ + if ((addr + (uint16_t)(n)) > buf_len) { \ + NDEF_MSG.success = NDEF_ERR_TRUNCATED; \ + return NDEF_MSG; \ + } \ + } +ndef_message readNDEFText(unsigned char *buf, uint8_t buf_len) { + uint16_t addr = 0; + NDEF_MSG.success = 0; + NDEF_MSG.payload_len = 0; + NDEF_MSG.payload[0] = '\0'; + + NDEF_NEED(2); if (buf[0] != NDEF_TLV) { NDEF_MSG.success = 1; return NDEF_MSG; @@ -18,6 +31,7 @@ ndef_message readNDEFText(unsigned char *buf) { // int len_field = buf[1]; addr = 2; + NDEF_NEED(3); // bool is_short_record = (buf[addr] & NDEF_SHORT_RECORD) == NDEF_SHORT_RECORD; bool has_id_length = (buf[addr] & NDEF_ID_LEN) == NDEF_ID_LEN; uint8_t tnf = buf[addr] & 0x7; @@ -31,18 +45,18 @@ ndef_message readNDEFText(unsigned char *buf) { uint8_t id_length = 0; if (has_id_length) { + NDEF_NEED(1); id_length = buf[addr]; addr += 1; } - uint8_t type_value[type_length + 1]; - for (uint8_t i = 0; i < type_length; i++) { - type_value[i] = buf[addr]; - addr += 1; // 6 - } + // Only the first type byte is ever inspected, so skip the rest rather than + // copying them into a tag-sized VLA. + NDEF_NEED(type_length); + uint8_t type_value_0 = (type_length > 0) ? buf[addr] : 0; + addr += type_length; - type_value[type_length] = 0; - if (type_value[0] != NDEF_TEXT_RECORD) { + if (type_value_0 != NDEF_TEXT_RECORD) { NDEF_MSG.success = 11; return NDEF_MSG; }; @@ -52,21 +66,39 @@ ndef_message readNDEFText(unsigned char *buf) { }; if (has_id_length && (id_length > 0)) { + NDEF_NEED(id_length); addr += id_length; } + NDEF_NEED(1); uint8_t lang_str_len = buf[addr]; + + // payload_length covers the language-length byte plus the language code + // plus the text. Subtracting without this check wraps a uint8_t to ~250. + if (payload_length < ((uint16_t)lang_str_len + 1)) { + NDEF_MSG.success = NDEF_ERR_BAD_LENGTH; + return NDEF_MSG; + } + payload_length -= lang_str_len; // Language string + payload_length -= 1; // The byte that says how long the language string is + + NDEF_NEED((uint16_t)lang_str_len + 1); addr += lang_str_len; addr += 1; - payload_length -= lang_str_len; // Language string - payload_length -= 1; // The byte that says how long the language string is + + // Leave room for the terminator the UART print and strchr() both rely on. + if (payload_length > (sizeof(NDEF_MSG.payload) - 1)) { + payload_length = sizeof(NDEF_MSG.payload) - 1; + } + NDEF_NEED(payload_length); + for (uint8_t i = 0; i < (payload_length); i++) { NDEF_MSG.payload[i] = buf[addr]; addr += 1; } - - // NDEF_MSG.payload = payload; + NDEF_MSG.payload[payload_length] = '\0'; NDEF_MSG.payload_len = payload_length; + #if DO_UART uart_sendString(NDEF_MSG.payload); uart_sendString("\n"); diff --git a/avr_code/ndef.h b/avr_code/ndef.h index 1fbd48b..337bf49 100644 --- a/avr_code/ndef.h +++ b/avr_code/ndef.h @@ -24,7 +24,11 @@ extern "C" { #define NDEF_TEXT_RECORD 0x54 #define TNF_KNOWN 0x01 -ndef_message readNDEFText(unsigned char *buf) ; +// readNDEFText failure codes reported through ndef_message.success +#define NDEF_ERR_TRUNCATED 13 +#define NDEF_ERR_BAD_LENGTH 14 + +ndef_message readNDEFText(unsigned char *buf, uint8_t buf_len); #ifdef __cplusplus } diff --git a/avr_code/power_mgmt.c b/avr_code/power_mgmt.c index 2e6ff9a..65d6833 100644 --- a/avr_code/power_mgmt.c +++ b/avr_code/power_mgmt.c @@ -1,6 +1,6 @@ #include "power_mgmt.h" -void shutdown_all_peripherals() { +void shutdown_all_peripherals(void) { power_adc_disable(); power_timer0_disable(); diff --git a/avr_code/power_mgmt.h b/avr_code/power_mgmt.h index b906017..631eb36 100644 --- a/avr_code/power_mgmt.h +++ b/avr_code/power_mgmt.h @@ -4,6 +4,7 @@ * * Created on December 20, 2024, 3:54 PM */ +#include "defines.h" // for DO_UART, which shutdown_all_peripherals() tests #include #ifndef POWER_MGMT_H #define POWER_MGMT_H @@ -13,7 +14,7 @@ extern "C" { #endif - void shutdown_all_peripherals(); + void shutdown_all_peripherals(void); #ifdef __cplusplus } diff --git a/avr_code/rfm69.c b/avr_code/rfm69.c index b5a406e..71401a9 100644 --- a/avr_code/rfm69.c +++ b/avr_code/rfm69.c @@ -1,8 +1,5 @@ #include "rfm69.h" -int8_t rssi; -uint8_t i; -uint8_t len_payload; uint32_t msg_hash; uint8_t p_hash_1; uint8_t p_hash_2; @@ -15,8 +12,6 @@ uint8_t c_hash_3; bool cond_1; bool cond_2; bool cond_3; -bool cond_4; -bool cond_5; DATA_SEND_STATUS send_message(tx_rx_data_struct tx_data) { @@ -46,19 +41,19 @@ DATA_SEND_STATUS send_message(tx_rx_data_struct tx_data) cond_3 = cond_1 && cond_2; if (cond_3 && (RX_DATA.flags == MSG_RECV_COUNTS_SUCCESS) && (RX_DATA.msg[3] == 0xFF)) { - #if DO_UART - uart_sendString(" RX DATA SUCCESS\n"); +#if DO_UART + uart_sendString(" RX DATA SUCCESS\n"); #endif return DATA_SEND_SUCCESS; - break; - } else if ( + } +#if DO_UART + else if ( cond_3 && (RX_DATA.flags == MSG_RECV_COUNTS_FAIL) && (RX_DATA.msg[3] == 0x00)) { -uart_sendString(" RX DATA FAILED\n"); + uart_sendString(" RX DATA FAILED\n"); } else { uart_sendString(" RX DATA ANOTHER ERROR\n"); } - - } else { +#endif } } return DATA_NOT_SENT; @@ -88,8 +83,11 @@ void uart_print_tx_rx_data(tx_rx_data_struct tx_rx_print) void rfm69_write_msg(tx_rx_data_struct txrxd) { - set_rfm69_rx_mode(); - set_rfm69_tx_mode(); + // TxStart is configured as FifoNotEmpty, so the radio begins transmitting + // the moment the first byte lands. Fill the FIFO from standby and only then + // switch to TX, otherwise the packet goes out ahead of its own payload. + set_rfm69_standby(); + spi_rfm69_select(true); spi_write(REG_FIFO | RFM69_SPI_WRITE); if (txrxd.len > (60)) { @@ -106,22 +104,30 @@ void rfm69_write_msg(tx_rx_data_struct txrxd) } spi_rfm69_select(false); + set_rfm69_tx_mode(); wait_tx_sent(); set_rfm69_rx_mode(); } -tx_rx_data_struct rfm69_read_msg() +tx_rx_data_struct rfm69_read_msg(void) { memset(RX_DATA.msg, ' ', sizeof(RX_DATA.msg)); spi_rfm69_select(true); spi_write(REG_FIFO); - RX_DATA.len = spi_read() - 4; + uint8_t raw_len = spi_read(); RX_DATA.to = spi_read(); RX_DATA.from = spi_read(); RX_DATA.dtype = spi_read(); RX_DATA.flags = spi_read(); - uint8_t len_f = RX_DATA.len; + + // The length byte comes off the air and is not trustworthy: below 4 it + // underflows to ~252, above 60 it walks off the end of msg[]. + uint8_t len_f = (raw_len < 4) ? 0 : (uint8_t)(raw_len - 4); + if (len_f > sizeof(RX_DATA.msg)) { + len_f = sizeof(RX_DATA.msg); + } + RX_DATA.len = len_f; for (uint8_t idx_f = 0; idx_f < len_f; idx_f++) { RX_DATA.msg[idx_f] = spi_read(); @@ -132,21 +138,11 @@ tx_rx_data_struct rfm69_read_msg() return RX_DATA; } -void rfm69_set_state(bool state) -{ - SET_PIN_OUT(DDRC, DDC2); - if (!state) { - SET_PIN_HIGH(PORTC, PC2); - } else { - SET_PIN_LOW(PORTC, PC2); - } -} - uint8_t spi_read_rfm69_rt(uint8_t reg) { spi_rfm69_select(true); spi_write(reg); - uint8_t data_read = spi_read(0xFF); + uint8_t data_read = spi_read(); spi_rfm69_select(false); return data_read; } @@ -221,23 +217,25 @@ uint32_t hash_3bytes(unsigned const char* str, uint8_t str_len) { uint32_t hash = 0; - for (i = 0; i < str_len; i++) { + for (uint8_t i = 0; i < str_len; i++) { hash = (hash * 31 + str[i]) % 0xFFFFFF; } return hash; } -void set_rfm69_power_amp_boost() +void set_rfm69_power_amp_boost(void) { + spi_write_rfm69_rt(REG_OCP, VAL_OCP_OFF); spi_write_rfm69_rt(REG_TEST_PA1, VAL_TEST_PA1_BOOST); spi_write_rfm69_rt(REG_TEST_PA2, VAL_TEST_PA2_BOOST); } -void set_rfm69_power_amp_normal() +void set_rfm69_power_amp_normal(void) { spi_write_rfm69_rt(REG_TEST_PA1, VAL_TEST_PA1_NORMAL); spi_write_rfm69_rt(REG_TEST_PA2, VAL_TEST_PA2_NORMAL); -}; + spi_write_rfm69_rt(REG_OCP, VAL_OCP_ON); +} void reset_txrx_struct(tx_rx_data_struct* s) { @@ -258,10 +256,15 @@ void set_rfm69_mode(uint8_t target_mode) spi_write_rfm69_rt(REG_OP_MODE, mode); } -void wait_tx_sent() +bool wait_tx_sent(void) { - while (TX_NOT_SENT) - ; + for (uint16_t attempts = 0; attempts < RFM69_TIMEOUT_MS; attempts++) { + if (TX_SENT) { + return true; + } + _delay_ms(1); + } + return false; } uint8_t hash(const char* str, uint8_t min, uint8_t max) @@ -279,68 +282,69 @@ uint8_t hash(const char* str, uint8_t min, uint8_t max) bool wait_rx_payload_ready_timeout(uint16_t attempts) { set_rfm69_rx_mode(); - uint16_t counter = 0; - while (1) { - _delay_ms(1); - WHILE_BREAK(counter, attempts); + // Test the flag before spending the tick, so a payload that arrives on the + // last attempt is not thrown away. + for (uint16_t counter = 0; counter < attempts; counter++) { if (RX_PAYLOAD_READY) { return true; - break; } + _delay_ms(1); + } + return RX_PAYLOAD_READY != 0; +} + +bool wait_rx_payload_ready(void) +{ + return wait_rx_payload_ready_timeout(RFM69_TIMEOUT_MS); +} + +bool wait_rfm69_mode_ready(void) +{ + for (uint16_t attempts = 0; attempts < RFM69_TIMEOUT_MS; attempts++) { + if (MODE_READY) { + return true; + } + _delay_ms(1); } return false; - - ; } -void wait_rx_payload_ready() -{ - - while (RX_PAYLOAD_NOT_READY) { }; -} - -void wait_rfm69_mode_ready() -{ - while (MODE_NOT_READY) - ; -} - -void set_rfm69_tx_mode() +void set_rfm69_tx_mode(void) { set_rfm69_power_amp_boost(); set_rfm69_mode(VAL_OPMODE_TX); wait_rfm69_mode_ready(); -}; +} -void set_rfm69_rx_mode() +void set_rfm69_rx_mode(void) { set_rfm69_power_amp_normal(); set_rfm69_mode(VAL_OPMODE_RX); wait_rfm69_mode_ready(); -}; +} -void set_rfm69_standby() +void set_rfm69_standby(void) { set_rfm69_power_amp_normal(); set_rfm69_mode(VAL_OPMODE_STDBY); wait_rfm69_mode_ready(); } -void set_rfm69_sleep() +void set_rfm69_sleep(void) { set_rfm69_power_amp_normal(); set_rfm69_mode(VAL_OPMODE_SLEEP); wait_rfm69_mode_ready(); } -void set_rfm69_idle() +void set_rfm69_idle(void) { set_rfm69_power_amp_normal(); set_rfm69_mode(VAL_OPMODE_STDBY); wait_rfm69_mode_ready(); } -void reset_rfm69() +void reset_rfm69(void) { rfm69_reset_state(true); _delay_ms(10); @@ -348,14 +352,7 @@ void reset_rfm69() _delay_ms(10); } -void set_rfm69_tx_power() -{ - uint8_t PA_LEVEL_SET - = VAL_PALEVEL_PA1_ON | VAL_PALEVEL_PA2_ON | ((20 + 14) & VAL_PALEVEL_PA1_OUTPUTPOWER); - spi_write_rfm69_rt(REG_PA_LEVEL, PA_LEVEL_SET); -} - -void rfm69_init() +void rfm69_init(void) { reset_rfm69(); _delay_ms(100); @@ -364,8 +361,6 @@ void rfm69_init() spi_write_rfm69_rt(REG_FREQ_MIDDLE_SB, VAL_FREQ_433MHz_MID_SB); spi_write_rfm69_rt(REG_FREQ_LSB, VAL_FREQ_433MHz_LSB); - spi_write_rfm69_rt(REG_FREQ_DEV_MSB, VAL_FREQ_DEV_MSB); - spi_write_rfm69_rt( REG_FIFO_THRESH, VAL_TX_START_FIFO_NOT_EMPTY | VAL_FIFO_LEVEL_INTERRUPT); // TX condition @@ -377,14 +372,15 @@ void rfm69_init() spi_write_rfm69_rt(REG_SYNC_CONFIG, VAL_SYNCWORDS_ON | VAL_SYNCWORDS_SIZE_2_BYTES); spi_write_rfm69_rt(REG_DATA_MODUL, - VAL_DATA_PACKET_MODE | VAL_DATA_MODUL_OOK); // RegDataModul + VAL_DATA_PACKET_MODE | VAL_DATA_MODUL_FSK + | VAL_MODUL_SHAPING_GAUSS_BT_1_0); // RegDataModul spi_write_rfm69_rt(REG_BITRATE_MSB, VAL_BITRATE_250kbps_MSB); // RegBitrateMSB spi_write_rfm69_rt(REG_BITRATE_LSB, VAL_BITRATE_250kbps_LSB); // RegbBitrateLSB - spi_write_rfm69_rt(REG_FDEV_MSB, 0x10); // RegFdevMSB - spi_write_rfm69_rt(REG_FDEV_LSB, 0x00); // RegFdevLSB + spi_write_rfm69_rt(REG_FDEV_MSB, VAL_FDEV_MSB); // RegFdevMSB (0x05) + spi_write_rfm69_rt(REG_FDEV_LSB, VAL_FDEV_LSB); // RegFdevLSB (0x06) spi_write_rfm69_rt(REG_RX_BW, 0xE0); // RegRxBw spi_write_rfm69_rt(REG_AFC_BW, 0xE0); // RegAfcBw diff --git a/avr_code/rfm69.h b/avr_code/rfm69.h index 0f84b86..b637152 100644 --- a/avr_code/rfm69.h +++ b/avr_code/rfm69.h @@ -15,14 +15,21 @@ #ifndef RFM69_H #define RFM69_H -#define MODE_NOT_READY !(spi_read_rfm69_rt(REG_IRQ_FLAGS1) & VAL_IRQ_FLAGS1_MODEREADY) -#define RX_PAYLOAD_READY spi_read_rfm69_rt(REG_IRQ_FLAGS2) & VAL_IRQ_FLAGS2_RX_PAYLOADREADY -#define RX_PAYLOAD_NOT_READY !(RX_PAYLOAD_READY) -#define TX_NOT_SENT !(spi_read_rfm69_rt(REG_IRQ_FLAGS2) & VAL_IRQ_FLAGS2_TX_SENT) #ifdef __cplusplus extern "C" { #endif +#define MODE_READY (spi_read_rfm69_rt(REG_IRQ_FLAGS1) & VAL_IRQ_FLAGS1_MODEREADY) +#define MODE_NOT_READY (!MODE_READY) +#define RX_PAYLOAD_READY (spi_read_rfm69_rt(REG_IRQ_FLAGS2) & VAL_IRQ_FLAGS2_RX_PAYLOADREADY) +#define RX_PAYLOAD_NOT_READY (!RX_PAYLOAD_READY) +#define TX_SENT (spi_read_rfm69_rt(REG_IRQ_FLAGS2) & VAL_IRQ_FLAGS2_TX_SENT) +#define TX_NOT_SENT (!TX_SENT) + +// Bail-out for every RFM69 poll loop: an absent or unpowered radio must not +// hang the firmware, since no watchdog reset is armed. +#define RFM69_TIMEOUT_MS 100U + #define REG_FIFO 0x00 #define REG_FREQ_MSB 0x07 #define REG_FREQ_MIDDLE_SB 0x08 @@ -35,7 +42,7 @@ extern "C" { #define REG_DATA_MODUL 0x02 #define REG_BITRATE_MSB 0x03 #define REG_BITRATE_LSB 0x04 -#define REG_FDEV_MSB 0x06 +#define REG_FDEV_MSB 0x05 #define REG_FDEV_LSB 0x06 #define REG_RX_BW 0x19 #define REG_AFC_BW 0x1A @@ -48,9 +55,15 @@ extern "C" { #define REG_IRQ_FLAGS1 0x27 #define REG_IRQ_FLAGS2 0x28 #define REG_RSSI_VALUE 0x24 +#define REG_OCP 0x13 -#define REG_FREQ_DEV_MSB 0x05 -#define VAL_FREQ_DEV_MSB 0x10 +// Over-current protection must be off while the PA boost registers are set, +// per the datasheet's high-power (+20 dBm) sequence. +#define VAL_OCP_OFF 0x0F +#define VAL_OCP_ON 0x1A + +#define VAL_FDEV_MSB 0x10 +#define VAL_FDEV_LSB 0x00 #define VAL_TEST_DAGC_DEFAULT 0x30 #define VAL_DATA_PACKET_MODE 0x00 @@ -58,7 +71,8 @@ extern "C" { #define VAL_BITRATE_250kbps_MSB 0x00 #define VAL_BITRATE_250kbps_LSB 0x80 -#define VAL_DATA_MODUL_OOK 0x01 +#define VAL_DATA_MODUL_FSK 0x00 // RegDataModul ModulationType is bits 4:3 +#define VAL_MODUL_SHAPING_GAUSS_BT_1_0 0x01 #define VAL_TX_START_FIFO_NOT_EMPTY 0x80 #define VAL_FIFO_LEVEL_INTERRUPT 0x0f @@ -104,28 +118,26 @@ extern "C" { #define VAL_FREQ_433MHz_LSB 0x00 DATA_SEND_STATUS send_message(tx_rx_data_struct tx_data); -void rfm69_set_state(bool state); uint8_t spi_read_rfm69_rt(uint8_t reg); uint8_t spi_write_rfm69_rt(uint8_t reg, uint8_t val); uint8_t spi_write_rfm69_multiple_rt(uint8_t reg, const char* vals, uint8_t len); -void set_rfm69_power_amp_boost(); -void set_rfm69_power_amp_normal(); -tx_rx_data_struct rfm69_read_msg(); +void set_rfm69_power_amp_boost(void); +void set_rfm69_power_amp_normal(void); +tx_rx_data_struct rfm69_read_msg(void); void reset_txrx_struct(tx_rx_data_struct* s); void rfm69_write_msg(tx_rx_data_struct txrxd); void set_rfm69_mode(uint8_t mode); -void wait_rfm69_mode_ready(); -void set_rfm69_tx_mode(); -void wait_tx_sent(); -void wait_rx_payload_ready(); -void reset_rfm69(); -void set_rfm69_rx_mode(); -void set_rfm69_standby(); -void set_rfm69_sleep(); -void set_rfm69_idle(); -void set_rfm69_tx_power(); -void rfm69_init(); +bool wait_rfm69_mode_ready(void); +void set_rfm69_tx_mode(void); +bool wait_tx_sent(void); +bool wait_rx_payload_ready(void); +void reset_rfm69(void); +void set_rfm69_rx_mode(void); +void set_rfm69_standby(void); +void set_rfm69_sleep(void); +void set_rfm69_idle(void); +void rfm69_init(void); bool wait_rx_payload_ready_timeout(uint16_t attempts); uint8_t hash(const char* str, uint8_t min, uint8_t max); uint32_t hash_3bytes(unsigned const char* str, uint8_t str_len); diff --git a/avr_code/spi.c b/avr_code/spi.c index 6855a38..e89f1c9 100644 --- a/avr_code/spi.c +++ b/avr_code/spi.c @@ -21,6 +21,6 @@ void spi_rfm69_select(bool state) return SPDR1; // Return received data } - uint8_t spi_read() { + uint8_t spi_read(void) { return spi_write(0xFF); } diff --git a/avr_code/spi.h b/avr_code/spi.h index a4eba49..ff6583a 100644 --- a/avr_code/spi.h +++ b/avr_code/spi.h @@ -7,7 +7,7 @@ uint8_t spi_write(uint8_t data); -uint8_t spi_read(); +uint8_t spi_read(void); void spi_rfm69_select(bool state); #endif \ No newline at end of file diff --git a/avr_code/st25dv.c b/avr_code/st25dv.c index dc57838..dd3e82d 100644 --- a/avr_code/st25dv.c +++ b/avr_code/st25dv.c @@ -1,43 +1,70 @@ #include "st25dv.h" -identifier_results get_nugget_data() { +#define IDENT_NAME_MAX (sizeof(IDENTIFIER.name_str) - 1) +#define IDENT_DIAM_MAX (sizeof(IDENTIFIER.diameter_str) - 1) + +static void set_identifier(const char* name, uint8_t name_len, const char* diam, uint8_t diam_len) { + if (name_len > IDENT_NAME_MAX) { + name_len = IDENT_NAME_MAX; + } + if (diam_len > IDENT_DIAM_MAX) { + diam_len = IDENT_DIAM_MAX; + } + memcpy(IDENTIFIER.name_str, name, name_len); + IDENTIFIER.name_str[name_len] = '\0'; + IDENTIFIER.name_len = name_len; + + memcpy(IDENTIFIER.diameter_str, diam, diam_len); + IDENTIFIER.diameter_str[diam_len] = '\0'; + IDENTIFIER.diameter_len = diam_len; + + IDENTIFIER.hashed = hash(IDENTIFIER.name_str, 0, 59); +} + +identifier_results get_nugget_data(void) { NDEF_MSG = rfid_read_first_ndef_entry(); + // readNDEFText reports parse failures through success; without this check a + // missing or malformed tag leaves stale/uninitialised bytes in payload and + // we transmit them as the node identity. + if (NDEF_MSG.success != 0) { +#if DO_UART + uart_print_uint8(NDEF_MSG.success, "NDEF parse failed, code "); +#endif + set_identifier("UNKNOWN", 7, "N/A", 3); + return IDENTIFIER; + } + TRIMMED_STRING = remove_spaces(NDEF_MSG.payload, NDEF_MSG.payload_len); char* delim_ptr = strchr(TRIMMED_STRING.str, ','); if (delim_ptr != NULL) { - uint8_t index_comma = delim_ptr - TRIMMED_STRING.str; - memcpy(IDENTIFIER.name_str, TRIMMED_STRING.str, index_comma); - memcpy( - IDENTIFIER.diameter_str, TRIMMED_STRING.str + index_comma + 1, TRIMMED_STRING.length); - IDENTIFIER.name_len = index_comma; - IDENTIFIER.diameter_len = TRIMMED_STRING.length - index_comma; + uint8_t index_comma = (uint8_t)(delim_ptr - TRIMMED_STRING.str); + // The diameter is what follows the comma, so its length is the + // remainder of the string -- not the whole string's length, which read + // off the end of the 21-byte buffer. + uint8_t diam_len = (uint8_t)(TRIMMED_STRING.length - index_comma - 1); + set_identifier(TRIMMED_STRING.str, index_comma, delim_ptr + 1, diam_len); } else { - memcpy(IDENTIFIER.name_str, TRIMMED_STRING.str, TRIMMED_STRING.length); - IDENTIFIER.name_len = TRIMMED_STRING.length; - memcpy(IDENTIFIER.diameter_str, "N/A", 3); - IDENTIFIER.diameter_len = 3; + set_identifier(TRIMMED_STRING.str, (uint8_t)TRIMMED_STRING.length, "N/A", 3); } - IDENTIFIER.hashed = hash(IDENTIFIER.name_str, 0, 59); - return IDENTIFIER; } trimmed_string_struct remove_spaces(char* str, uint8_t len_str) { - uint8_t i = 0, j = 0; - memset(TRIMMED_STRING.str, ' ', 20); - while (str[i]) { - if (str[i] != ' ') { + const uint8_t max_len = sizeof(TRIMMED_STRING.str) - 1; + uint8_t j = 0; + + memset(TRIMMED_STRING.str, 0, sizeof(TRIMMED_STRING.str)); + for (uint8_t i = 0; (i < len_str) && str[i]; i++) { + if ((str[i] != ' ') && (j < max_len)) { TRIMMED_STRING.str[j++] = str[i]; } - i++; - if (i >= len_str) { - break; - } } + // Callers run strchr() over this, so it has to be terminated. + TRIMMED_STRING.str[j] = '\0'; TRIMMED_STRING.length = j; return TRIMMED_STRING; } @@ -51,25 +78,26 @@ void rfid_set_low_power_down(bool state) { } } -ndef_message rfid_read_first_ndef_entry() { +ndef_message rfid_read_first_ndef_entry(void) { rfid_set_low_power_down(false); rfid_set_i2c_power(true); _delay_ms(1); - memset(DATA_BUFFER_65, ' ', 64); + // static: a 65-byte frame here sat on top of an already deep call chain and + // was a large part of the stack overrun. + static unsigned char DATA_BUFFER_INTERNAL[NDEF_READ_LEN]; + memset(DATA_BUFFER_INTERNAL, 0, sizeof(DATA_BUFFER_INTERNAL)); + rfid_read_memory(DATA_BUFFER_INTERNAL, NDEF_READ_LEN, 0x0000 + 4); - char DATA_BUFFER_INTERNAL[65]; - rfid_read_memory(DATA_BUFFER_INTERNAL, 64, 0x0000 + 4); + NDEF_MSG = readNDEFText(DATA_BUFFER_INTERNAL, NDEF_READ_LEN); - NDEF_MSG = readNDEFText(DATA_BUFFER_INTERNAL); - rfid_set_low_power_down(true); rfid_set_i2c_power(false); return NDEF_MSG; } -uint8_t rfid_read_system_register() { +uint8_t rfid_read_system_register(void) { return read_one_byte_16bit_addr_no_err_register(I2C_SYSTEM_ADDR, 0x0000); } diff --git a/avr_code/st25dv.h b/avr_code/st25dv.h index b97564a..0478155 100644 --- a/avr_code/st25dv.h +++ b/avr_code/st25dv.h @@ -13,12 +13,15 @@ #define I2C_SYSTEM_ADDR 0x57 #define I2C_USER_ADDR 0x53 -identifier_results get_nugget_data(); -ndef_message rfid_read_first_ndef_entry(); +// Bytes of tag memory pulled in one go to look for the first NDEF record. +#define NDEF_READ_LEN 64 + +identifier_results get_nugget_data(void); +ndef_message rfid_read_first_ndef_entry(void); void rfid_set_low_power_down(bool state); void rfid_set_i2c_power(bool state); uint8_t rfid_read_memory(uint8_t* data, uint8_t num_bytes, uint16_t address); -uint8_t rfid_read_system_register(); +uint8_t rfid_read_system_register(void); trimmed_string_struct remove_spaces(char* str, uint8_t len_str); diff --git a/avr_code/states.c b/avr_code/states.c index 93ef807..5c40b21 100644 --- a/avr_code/states.c +++ b/avr_code/states.c @@ -1,11 +1,17 @@ #include "states.h" - void init_spi() { - SET_PIN_OUT(DDRC, DDC1); // SCK - SET_PIN_OUT(DDRE, DDE3); // MOSI - SET_PIN_IN(DDRC, DDC0); // MISO_RFM69 - SET_PIN_HIGH(PORTC, PC0); - SPCR1= (1< f_osc/4 } diff --git a/avr_code/states.h b/avr_code/states.h index d175e98..e57b09c 100644 --- a/avr_code/states.h +++ b/avr_code/states.h @@ -24,7 +24,7 @@ extern "C" { #endif - void init_spi(); + void init_spi(void); void rfm69_reset_state(bool state) ; void led_1_set_state(bool state); diff --git a/avr_code/uart.h b/avr_code/uart.h index 297b5cb..bc65316 100644 --- a/avr_code/uart.h +++ b/avr_code/uart.h @@ -6,7 +6,6 @@ */ #include "defines.h" -#define UBRR_BAUD F_CPU / 16 / BAUD - 1 #include #include @@ -22,7 +21,9 @@ extern "C" { #endif -void uart_init(); +#define UBRR_BAUD ((F_CPU) / 16 / (BAUD) - 1) + +void uart_init(void); void uart_sendChar(char c); void uart_sendString(const char* str); void uart_sendStringArray(unsigned char str[], uint8_t len); @@ -34,7 +35,7 @@ void uart_print_float(float meas, const char* buf); void uart_print_binary(unsigned char vin, const char* buf); void uart_print_uint8(uint8_t vin, const char* buf); void uart_print_uint8_array(uint8_t* array, size_t length, const char* buf); -void uart_wait_until_sent(); +void uart_wait_until_sent(void); #ifdef __cplusplus }