modbus.c rewritten for a single buffer

No need for separate Rx and Tx buffer with request-response nature
of MODBUS/RTU.

diff --git a/firmware/modbus.c b/firmware/modbus.c
index 2b2a255bedc15d8523d30a9b5d8d518421de5331..4e8456354e1a5385d630eb7f4cd59cff0f6606a3 100644 (file)
--- a/firmware/modbus.c
+++ b/firmware/modbus.c
@@ -14,28 +14,21 @@
 #include "clock.h"
 #include "modbus.h"
 
-#define BUFSIZE 128    // must be a power of two
+#define BUFSIZE 128 // maximum request size
 
 // configure the control pin
 #define ctl_pin_setup()        do { DDRD  |=  _BV(PD2); } while (0)
 #define ctl_pin_on()   do { PORTD |=  _BV(PD2); } while (0)
 #define ctl_pin_off()  do { PORTD &= ~_BV(PD2); } while (0)
 
-#define BUFMASK (BUFSIZE-1)
-#if (BUFSIZE & BUFMASK)
-#error BUFSIZE must be a power of two
-#endif
-
 #if BUFSIZE > 255
 typedef uint16_t bufptr_t;
 #else
 typedef uint8_t  bufptr_t;
 #endif
 
-#define bufptr_inc(x)  ((x + 1) & BUFMASK)
-
-static volatile bufptr_t rx_bytes, tx_head, tx_tail;
-static volatile uint8_t rxbuf[BUFSIZE], txbuf[BUFSIZE];
+static volatile bufptr_t buf_len, tx_ptr;
+static volatile uint8_t buffer[BUFSIZE], transmitting;;
 static volatile uint16_t last_rx;
 #ifndef mb_unit_id
 static uint8_t mb_unit_id;
@@ -64,8 +57,8 @@ static uint16_t hold_regs_ee[MB_N_HOLD_REGS_EEPROM] EEMEM = {
 
 void modbus_init(uint8_t unit)
 {
-       rx_bytes = 0;
-       tx_head = tx_tail = 0;
+       buf_len = 0;
+       transmitting = 0;
 
        if (unit)
                mb_unit_id = unit;
@@ -80,31 +73,13 @@ void modbus_init(uint8_t unit)
        ctl_pin_off();
        ctl_pin_setup();
 
+       // Serial port setup
        UBRR0 = UBRR_VAL;
        UCSR0A = 0;
        UCSR0B = _BV(RXCIE0)|_BV(RXEN0)|_BV(TXEN0);
        UCSR0C = _BV(UCSZ01)|_BV(UCSZ00);
 }
 
-void rs485_send(char *p)
-{
-       bufptr_t next;
-
-       if (*p == '\0')
-               return;
-
-       ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
-               next = bufptr_inc(tx_head);
-               while (next != tx_tail && *p != '\0') {
-                       txbuf[tx_head] = *p++;
-                       tx_head = next;
-                       next = bufptr_inc(tx_head);
-               }
-               ctl_pin_on();
-               UCSR0B |= _BV(UDRIE0);
-       }
-}
-
 static uint16_t compute_crc(volatile uint8_t *buf, bufptr_t len)
 {
        bufptr_t i;
@@ -128,21 +103,21 @@ static uint16_t compute_crc(volatile uint8_t *buf, bufptr_t len)
 
 static void make_exception(mb_exception code)
 {
-       txbuf[1] |= 0x80;
-       txbuf[2] = code;
-       tx_head = 3;
+       buffer[1] |= 0x80;
+       buffer[2] = code;
+       buf_len = 3;
 }
 
 #define get_word(ptr, off) (((uint16_t)ptr[off] << 8) | ptr[off+1])
 void put_byte(uint8_t byte)
 {
-       txbuf[tx_head++] = byte;
+       buffer[buf_len++] = byte;
 }
 
 void put_word(uint16_t word)
 {
-       txbuf[tx_head++] = word >> 8;
-       txbuf[tx_head++] = word & 0xFF;
+       buffer[buf_len++] = word >> 8;
+       buffer[buf_len++] = word & 0xFF;
 }
 
 static mb_exception read_holding_regs(uint16_t start, uint16_t len)
@@ -191,97 +166,105 @@ void modbus_poll()
        uint8_t rv;
 
        ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
-               if (rx_bytes == 0) // nothing received yet
+               if (transmitting)
                        return;
 
-               if (get_clock() - last_rx < REQ_TIMEOUT) // still receiving
+               if (buf_len == 0) // nothing received yet
                        return;
 
-               if (rx_bytes < 4) { // too short
-                       rx_bytes = 0;
+               if (get_clock() - last_rx < REQ_TIMEOUT) // still receiving
                        return;
-               }
 
-               if (rxbuf[0] != mb_unit_id) { // not for myself
-                       rx_bytes = 0;
+               if (buf_len < 4) { // too short
+                       buf_len = 0;
                        return;
                }
 
-               if (tx_tail) { // still sending?
-                       rx_bytes = 0;
+               if (buffer[0] != mb_unit_id) { // not for myself
+                       buf_len = 0;
                        return;
                }
 
-               packet_len = rx_bytes; // make a copy
+               transmitting = 1; // disable further reads
+               packet_len = buf_len;
        }
 
-       crc = compute_crc(rxbuf, packet_len - 2);
+       crc = compute_crc(buffer, packet_len - 2);
 
-       if ((crc & 0xFF) != rxbuf[packet_len-2]
-               || (crc >> 8) != rxbuf[packet_len-1]) // bad crc
-               goto out;
+       if ((crc & 0xFF) != buffer[packet_len-2]
+               || (crc >> 8) != buffer[packet_len-1]) { // bad CRC
+               ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
+                       transmitting = 0;
+                       buf_len = 0;
+               }
+               return;
+       }
 
-       txbuf[0] = rxbuf[0]; // not mb_unit_id in case it gets changed
-       txbuf[1] = rxbuf[1];
-       tx_head = 2;
+       packet_len -= 2; // strip the CRC
+       buf_len = 2; // keep the first two bytes (unit ID and function) for TX
 
-       rv = MB_OK;
-       switch (rxbuf[1]) { // function
+       rv = MB_ILLEGAL_FUNC;
+
+       switch (buffer[1]) { // function
        case 3:
-               rv = read_holding_regs(get_word(rxbuf, 2), get_word(rxbuf, 4));
+               if (packet_len == 5)
+                       rv = read_holding_regs(
+                               get_word(buffer, 2),
+                               get_word(buffer, 4)
+                       );
                break;
        case 6:
-               rv = write_single_reg(get_word(rxbuf, 2), get_word(rxbuf, 4));
+               if (packet_len == 5)
+                       rv = write_single_reg(
+                               get_word(buffer, 2),
+                               get_word(buffer, 4)
+                       );
                break;
-       default:
-               make_exception(MB_ILLEGAL_FUNC); // illegal function
        }
        
        if (rv)
                make_exception(rv);
-send:
-       if (tx_head) {
-               crc = compute_crc(txbuf, tx_head);
-               txbuf[tx_head++] = crc & 0xFF;
-               txbuf[tx_head++] = crc >> 8;
 
-               tx_tail = 0;
+       // append the CRC
+       crc = compute_crc(buffer, buf_len);
+       put_byte(crc & 0xFF);
+       put_byte(crc >> 8);
 
-               ctl_pin_on();
-               UCSR0B |= _BV(UDRIE0);
-       }
-out:
-       ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
-               rx_bytes = 0;
-       }
+       // send out the reply
+       tx_ptr = 0;
+       ctl_pin_on();
+       UCSR0B |= _BV(UDRIE0);
 }
 
 ISR(USART_RX_vect)
 {
-       ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
-               rxbuf[rx_bytes] = UDR0;
-               if (rx_bytes + 1 < BUFSIZE) // ignore overruns
-                       rx_bytes++;
-               last_rx = get_clock();
-       }
+       uint8_t rx_byte = UDR0;
+
+       if (transmitting) // discard it
+               return;
+
+       buffer[buf_len] = rx_byte;
+
+       if (buf_len + 1 < BUFSIZE) // ignore overruns
+               buf_len++;
+
+       last_rx = get_clock();
 }
 
 ISR(USART_TX_vect)
 {
        UCSR0B &= ~_BV(TXCIE0); // disable further IRQs
        ctl_pin_off();
+       buf_len = 0;
+       transmitting = 0; // enable receiving
 }
 
 ISR(USART_UDRE_vect)
 {
-       ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
-               if (tx_head == tx_tail) {
-                       UCSR0B |= _BV(TXCIE0); // enable xmit complete irq
-                       UCSR0B &= ~_BV(UDRIE0);
-                       tx_tail = tx_head = 0;
-               } else {
-                       UDR0 = txbuf[tx_tail];
-                       tx_tail = bufptr_inc(tx_tail);
-               }
+       if (tx_ptr+1 >= buf_len) {
+               UCSR0B |= _BV(TXCIE0); // enable xmit complete irq
+               UCSR0B &= ~_BV(UDRIE0); // disable ourselves
+       } else {
+               UDR0 = buffer[tx_ptr++];
        }
 }