cmd: fix parse_hex_nibble(), better output, W command

Most uart code is now in `uart_tx.S`.
Code close to the hardware stays in `uart.c` to use `io.h`.
The .S does not use the CPP, yet.

src/Makefile

@@ -9,12 +9,11 @@ all: $(PROJ).hex
 SN_bate = 1
 MCU_bate = attiny424
 C_FILES_bate = uart.c rtc.c spi.c adc.c calib.c mul.c cmd.c
-
-BATE_PERIOD=76
-bate_CFLAGS = -DPERIOD=$(BATE_PERIOD)
+S_FILES_bate = uart_tx.S
 
 MCU = $(MCU_$(PROJ))
-OPT = -Os
+# When flash gets tight, use `OPT=-Os`, or use more assembler :-)
+OPT = -O2
 
 CC=avr-gcc -Wall -Wno-parentheses -MMD -std=c99 $(OPT) \
   -mmcu=$(MCU) \
@@ -30,7 +29,8 @@ SN = $(SN_$(PROJ))
 CFLAGS = $($*_CFLAGS) $(DEBUG) -I. -DSN="$(SN)"
 
 C_FILES = $(C_FILES_$(PROJ))
-OBJS = $(patsubst %.c, %.o, $(C_FILES))
+S_FILES = $(S_FILES_$(PROJ))
+OBJS = $(patsubst %.c, %.o, $(C_FILES)) $(patsubst %.S, %.o, $(S_FILES))
 
 %.s: %.c %.o
 	$(CC) $(CFLAGS) -S $<
@@ -38,6 +38,9 @@ OBJS = $(patsubst %.c, %.o, $(C_FILES))
 %.o: %.c
 	$(CC) -g $(CFLAGS) -c $<
 
+%.o: %.S
+	$(CC) -g $(CFLAGS) -c $<
+
 -include *.d
 
 LDFLAGS = -Teeprom.ld
@@ -102,7 +105,7 @@ ad: $(PROJ).ad
 	$(AD) -v -t
 
 %.burn: %.hex
-	$(AD) -U flash:w:$<
+	$(AD) -U flash:v:$< || $(AD) -U flash:w:$<
 
 %.verify: %.hex %.eeprom
 	-$(AD) -qq -U fuses:v:"$(fuses_$*)":m

src/bate.c

@@ -412,9 +412,9 @@ int main()
 
 	uint8_t reset_source = RSTCTRL.RSTFR;
 	RSTCTRL.RSTFR = reset_source;
-	send_str("\nV Turbo Weather V0.04\nR ");
+	send_str("\nV Turbo Weather V0.05\nB ");
 	send_hex_byte(reset_source);
-	send_char('\n');
+	send_eol();
 
 	uint8_t test_calib = config.calib_test;
 	if (test_calib > N_TESTDATA)
@@ -447,7 +447,7 @@ int main()
 				send_char('V');
 				for (uint8_t i=0; i<n_adc; i++)
 					send_calib_adc(i);
-				send_char('\n');
+				send_eol();
 			}
 		}
 
@@ -531,12 +531,12 @@ int main()
 			start_adc();
 
 		if (config.send & SEND_CLOCK) {
+			send_str("T 0x");
 			cli();
 			uint32_t time = clock;
 			sei();
-			send_str("T 0x");
 			send_hex_long(time);
-			send_char('\n');
+			send_eol();
 		}
 
 		uint8_t send_test = 0;

src/cmd.c

@@ -13,11 +13,10 @@ void parse_command(uint8_t *s, uint8_t n) {}
 __attribute__ ((noinline, noclone))
 uint8_t parse_hex_nibble(uint8_t c)
 {
-	if (c>='0' && c<='9')
+	if (c >= '0' && c <= '9')
 		return c - '0';
-	c |= 0x20;
-	if (c >= 'a' && c<= 'f')
-		return c - 'a';
+	if (c >= 'a' && c <= 'f')
+		return c - 'a' + 0x0a;
 	return 0xf0;
 }
 
@@ -55,24 +54,31 @@ void parse_command(uint8_t *s, uint8_t n)
 		rx_params[p++] = (h<<4) | d;
 	}
 
-	const char *m = "R?\n";
 	if (n && *s != '\n')
 		goto fail;
 	
-	if (cmd == 'C' && p>=2 && rx_params[0] < sizeof(struct config)) {
-		memcpy((uint8_t*)&config + rx_params[0], rx_params+1, p-1);
-		m = "RC!\n";
-	}
-	else if (cmd == 'M' && p==2) {
-		send_str("RM! ");
-		uint16_t a = (uint16_t)rx_params[0] << 8;
-		a |= rx_params[1];
-		send_hex_byte(*(uint8_t*)a);
-		m = "\n";
-	}
+	uint8_t *a = (uint8_t *)(((uint16_t)rx_params[0] << 8) | rx_params[1]);
 
+	if (cmd == 'C' && p >= 2 && rx_params[0] + p <= sizeof(struct config))
+		memcpy((uint8_t*)&config + rx_params[0], rx_params+1, p-1);
+	else if (cmd == 'M' && p==2) {
+		rx_params[2] = *a;
+		p++;
+	}
+	else if (cmd == 'W' && p==3) {
+		rx_params[3] = *a;
+		*a = rx_params[2];
+		p++;
+	}
+	else
+		goto fail;
+
+	send_char('R');
+	send_char('!');
+	send_hex(cmd, rx_params, p, 1);
+	return;
 fail:
-	send_str(m);
+	send_str("R?\n");
 }
 
 #endif

src/uart.c

@@ -58,12 +58,12 @@ uint8_t uart_tick()
 }
 
 uint8_t uart_tx[128];
+#define uart_tx_m (sizeof(uart_tx) - 1)
 volatile uint8_t uart_tx_w;
 volatile uint8_t uart_tx_r;
 volatile uint8_t uart_tx_busy;
-static const uint8_t uart_tx_m = sizeof(uart_tx) - 1;
 
-#if 1
+#if 0
 
 __attribute__ ((noinline, noclone))
 void tx()
@@ -108,7 +108,7 @@ void tx()
 	// [STATUS] is prepared in advance, to avoid two branches and one memory load,
 	// at the cost of one register on the ISR stack (r25)
 	__asm__("\n"
-		"	lds	r18, uart_tx_r				\n"
+		"	lds	r21, uart_tx_r				\n"
 		"	lds	r19, uart_tx_w				\n"
 		"	lds 	r25, %[CTRLA]				\n"
 		"	andi 	r25, ~0x20	; clr DREIE		\n"
@@ -122,8 +122,8 @@ void tx()
 		"	rjmp 2f						\n"
 		"							\n"
 		"1:							\n"
-		"	mov	r30, r18	;			\n"
-		"	subi 	r18, 0xff	; r++ & uart_tx_m	\n"
+		"	mov	r30, r21	;			\n"
+		"	subi 	r21, 0xff	; r++ & uart_tx_m	\n"
 		"	andi	r30, 0x7f	;			\n"
 		"	ldi 	r31, 0					\n"
 		"	subi 	r30, lo8(-(uart_tx))			\n"
@@ -133,7 +133,7 @@ void tx()
 		"	ldi	r24, 0x40				\n"
 		"	sts	%[STATUS], r24	; clr TXCIF		\n"
 		"2:							\n"
-		"	cp	r18, r19				\n"
+		"	cp	r21, r19				\n"
 		"	breq 3f						\n"
 		"	sts 	uart_tx_busy, r24	; =0x40		\n"
 		"	lds 	r24, %[STATUS]				\n"
@@ -142,12 +142,12 @@ void tx()
 		"	ori	r25, 0x60	; set DREIE TXCIE	\n"
 		"3:							\n"		
 		"	sts 	%[CTRLA], r25				\n"
-		"	sts 	uart_tx_r, r18				\n"
+		"	sts 	uart_tx_r, r21				\n"
 		:
 		: [STATUS] "n" (&USART0.STATUS),
 		  [CTRLA]  "n" (&USART0.CTRLA),
 		  [TXDATA] "n" (&USART0.TXDATAL)
-		: "r18", "r19",
+		: "r19", "r21",
 		  "r24", "r25",
 		  "r30", "r31",
 		  "memory"
@@ -165,8 +165,8 @@ ISR(USART0_DRE_vect, ISR_NAKED)
 		"	in 	r24, __SREG__			\n"
 		"	push	r24				\n"
 		"	push	r25				\n"
-		"	push	r18				\n"
 		"	push	r19				\n"
+		"	push	r21				\n"
 		"	push	r30				\n"
 		"	push	r31				\n"
 		"						\n"
@@ -179,12 +179,13 @@ ISR(USART0_DRE_vect, ISR_NAKED)
 		"						\n"
 		"	pop	r31				\n"
 		"	pop	r30				\n"
+		"	pop	r21				\n"
 		"	pop	r19				\n"
-		"	pop	r18				\n"
 		"	pop	r25				\n"
 		"	pop	r24				\n"
 		"	out	__SREG__, r24			\n"
 		"	pop	r24				\n"
+		"	reti					\n"
 		);
 }
 
@@ -192,16 +193,92 @@ ISR(USART0_DRE_vect, ISR_NAKED)
 
 ISR(USART0_TXC_vect, ISR_ALIASOF(USART0_DRE_vect));
 
-__attribute__ ((noinline, noclone))
-uint8_t uart_busy()
-{
-	cli();
-	tx();
-	sei();
-	return uart_tx_busy;
-}
+uint8_t uart_rx[16];
+#define uart_rx_m (sizeof(uart_rx) - 1)
 
-static inline
+volatile uint8_t uart_rx_w;
+volatile uint8_t uart_rx_mes;
+
+#if 0
+ISR(USART0_RXC_vect)
+{
+	DEBUG_COUNTER(rx_irqs);
+	// s/while/if/ !
+	if (USART0.STATUS & USART_RXCIF_bm) {
+		uint8_t c = USART0.RXDATAL;
+		DEBUG_POKE(rx_char, c);
+		uint8_t w = uart_rx_w;
+		uart_rx[w] = c;
+		if (w < uart_rx_m)
+			uart_rx_w = ++w;
+		if (!uart_rx_mes && c=='\n')
+			uart_rx_mes = w;
+	}
+}
+#else
+ISR(USART0_RXC_vect, ISR_NAKED)
+{
+	// The loop will in all cases execute exactly once.
+	// No need to avoid load and stores inside the loop.
+	// The volatileness is also irrelevant, we are the ISR,
+	// nobody else is looking.
+	// We could just have the ISR be called again
+	// in the unusual case there is backlog in the FIFO,
+	// and not loop here.
+	// We may not even need to test the RXCIF bit.
+	// Let's do that …
+	__asm__(
+		"	push	r24				\n"
+		"	in 	r24, __SREG__			\n"
+		"	push	r24				\n"
+		"	push	r30				\n"
+		"	push	r31				\n"
+#ifdef DEBUG
+		"	lds 	r24, debug_data + 4		\n"
+		"	subi 	r24, -1				\n"
+		"	sts 	debug_data + 4, r24		\n"
+#endif
+		"	lds	r24, %[STATUS]	 		\n"
+		"	sbrs	r24, %[RXCIF]			\n"
+		"	rjmp 1f					\n"
+		"	lds 	r24, %[RXDATA]	 		\n"
+		"	lds 	r30, uart_rx_w			\n"
+		"	ldi 	r31, 0		 		\n"
+		"	subi 	r30, lo8(-(uart_rx)) 		\n"
+		"	sbci 	r31, hi8(-(uart_rx)) 		\n"
+		"	st 	Z+, r24		 		\n"
+#ifdef DEBUG
+		"	sts 	debug_data + 5, r24		\n"
+#endif
+		"	subi	r30, lo8(uart_rx)		\n"
+		"	sbrs 	r30, 4		  		\n"
+		"	sts 	uart_rx_w, r30			\n"
+		"	cpi 	r24, '\n'	 		\n"
+		"	brne 1f			 		\n"
+		"	lds	r24, uart_rx_mes		\n"
+		"	tst	r24		 		\n"
+		"	brne 1f					\n"
+		"	sts	uart_rx_mes, r30		\n"
+		"1:						\n"
+		"	pop	r31				\n"
+		"	pop	r30				\n"
+		"	pop	r24				\n"
+		"	out	__SREG__, r24			\n"
+		"	pop	r24				\n"
+		"	reti					\n"
+		:
+		: [STATUS] "n" (&USART0.STATUS),
+		  [RXCIF]  "n" (USART_RXCIF_bp),
+		  [RXDATA] "n" (&USART0.RXDATAL)
+		);
+}
+#endif
+
+
+#if 0
+// These are implemented in `uart_tx.S`
+
+__attribute__ ((noinline, noclone))
 uint8_t uart_put_char(uint8_t c)
 {
 	uint8_t r = uart_tx_r;
@@ -245,50 +322,34 @@ void send_str(const char *s)
 	uart_busy();
 }
 
-uint8_t uart_rx[16];
-volatile uint8_t uart_rx_w;
-volatile uint8_t uart_rx_n;
-static const uint8_t uart_rx_m = sizeof(uart_rx) - 1;
-volatile uint8_t uart_rx_mes;
-
-ISR(USART0_RXC_vect)
-{
-	DEBUG_COUNTER(rx_irqs);
-	uint8_t w = uart_rx_w;
-	uint8_t n = uart_rx_n;
-	while (USART0.STATUS & USART_RXCIF_bm) {
-		uint8_t c = USART0.RXDATAL;
-		uart_rx[w] = c;
-		n++;
-		if (w < uart_rx_m)
-			w++;
-		if (c=='\n')
-			uart_rx_mes = w;
-		DEBUG_POKE(rx_char, c);
-	}
-	uart_rx_w = w;
-	uart_rx_n = n;
-}
-
 static inline
 void rx_dismiss(uint8_t n)
 {
 	cli();
 	uint8_t i = 0;
-	uart_rx_mes = 0;
+	uint8_t m = 0;
 	uint8_t w = uart_rx_w;
 	if (w != uart_rx_m)
 		while (n < w) {
 			uint8_t c = uart_rx[n++];
 			uart_rx[i++] = c;
-			if (c=='\n')
-				uart_rx_mes = i;
+			if (!m && c=='\n')
+				m = i;
 		}
+	uart_rx_mes = m;
 	uart_rx_w = i;
-	uart_rx_n = i;
 	sei();	
 }
 
+__attribute__ ((noinline, noclone))
+uint8_t uart_busy()
+{
+	cli();
+	tx();
+	sei();
+	return uart_tx_busy;
+}
+
 static __attribute__ ((noinline, noclone))
 void send_hex_nibble(uint8_t b)
 {
@@ -312,13 +373,6 @@ void send_hex_word(uint16_t b)
 	send_hex_byte(b);
 }
 
-__attribute__ ((noinline, noclone))
-void send_hex_long(uint32_t b)
-{
-	send_hex_word(b >> 16);
-	send_hex_word(b);
-}
-
 __attribute__ ((noinline, noclone))
 void send_hex(uint8_t header, const uint8_t *s, uint8_t n, uint8_t words)
 {
@@ -356,3 +410,5 @@ void command(void)
 		parse_command(uart_rx, m);
 	rx_dismiss(m);
 }
+
+#endif

src/uart.h

@@ -9,10 +9,6 @@
 
 void init_uart(uint16_t div, uint8_t mode);
 uint8_t uart_tick();
-void send_char(uint8_t c);
-void send_str(const char *s);
-uint8_t uart_busy();
-void command(void);
 
 static inline
 uint8_t uart_break_p()
@@ -20,10 +16,22 @@ uint8_t uart_break_p()
 	return !(VPORTB.IN & 0x08);
 }
 
+void send_char(uint8_t c);
+void send_eol();
+void send_str(const char *s);
 void send_hex_byte(uint8_t b);
 void send_hex_word(uint16_t b);
-void send_hex_long(uint32_t b);
 void send_hex(uint8_t header, const uint8_t *s, uint8_t n, uint8_t words);
+uint8_t uart_busy();
+void command(void);
+
 void send_decimal(uint16_t b,  uint8_t dec);
 
 void parse_command(uint8_t *s, uint8_t n);
+
+static inline
+void send_hex_long(uint32_t b)
+{
+	send_hex_word(b >> 16);
+	send_hex_word(b);
+}

src/uart_tx.S

@@ -0,0 +1,173 @@
+// avoid quite a few push and pops and jumps
+
+	.global send_hex_word
+	.global send_hex_byte
+	.global send_char
+	.global send_str
+	.global uart_busy
+	.global send_hex
+	.global send_eol
+	.global	command
+
+// `tx()` and `put_char()` do not gobble r18, r20, r23, r23, r26, and r27.
+	
+put_char:
+	// non-global, non-C
+	// arg: char r22 
+	lds 	r23, uart_tx_r
+	lds 	r30, uart_tx_w
+	ldi	r19, 1
+	add	r19, r30
+	eor	r23, r19
+	andi	r23, 0x7f // uart_tx_m
+	breq 1f
+	andi 	r30, 0x7f // uart_tx_m
+	ldi	r31, 0
+	subi 	r30, lo8(-(uart_tx))
+	sbci 	r31, hi8(-(uart_tx))
+	st 	Z, r22
+	sts 	uart_tx_w, r19
+1:
+	// r22 preserved
+	// r23 full when zero
+	ret
+
+	// r18, r20, r26, and 27 must be preseved in the hex functions
+	// r22 and r23 must be preserved in `uart_busy()` and `put_char()'
+
+send_hex_word:
+	push	r24
+	mov	r24, r25
+	rcall	send_hex_byte
+	pop	r24
+send_hex_byte:
+	push	r24
+	swap	r24
+	rcall	send_hex_nibble
+	pop	r24
+send_hex_nibble:
+	andi	r24, 0x0f
+	subi 	r24, -'0'
+	cpi 	r24, '9'+1
+	brlo	send_char
+	subi	r24, '9'+1-'A'
+
+send_char:
+	mov	r22, r24
+send_char22:	
+	rcall	put_char
+	rcall	uart_busy
+	tst	r23
+	brne 9f
+	sleep
+	rjmp	send_char22
+
+2:
+	rcall	put_char
+	tst	r23
+	brne 1f
+	rcall	send_char22
+1:
+	movw	r24, r26
+send_str:
+	movw	r26, r24
+	ld 	r22, X+
+	tst	r22
+	brne 2b
+uart_busy:
+	cli
+	rcall	tx	; gobbles only r19, r21, r24, r25, r30, and r31
+	sei	
+	lds	r24, uart_tx_busy
+9:
+	ret
+
+send_hex:
+	movw	r26, r22
+	rcall	send_char
+	rjmp	3f
+
+1:
+	ldi	r22, ' '
+	sbrc	r18, 0
+	rcall	send_char22
+	ld	r24, X+
+	sbrs	r18, 1
+	rjmp 2f
+	sbrs	r20, 0
+	rjmp 2f
+	push	r24
+	subi	r20, 1
+	ld	r24, X+
+	rcall	send_hex_byte
+	pop	r24
+2:	
+	rcall	send_hex_byte
+3:	
+	subi	r20, 1
+	brcc	1b
+send_eol:
+	ldi	r22, 10
+	rjmp	send_char22
+
+command:
+	lds	r20, uart_rx_mes
+	tst	r20
+	breq 9b
+	ldi	r22, 'R'
+	rcall	send_char22
+	ldi	r22, ' '
+	rcall	send_char22
+	ldi	r26, lo8(uart_rx)
+	ldi	r27, hi8(uart_rx)
+	mov	r18, r20
+1:
+	ld	r22, X+
+	call	send_char22
+	subi	r18, 1
+	brcc	1b
+	cpi	r22, '\n'
+	breq 2f
+	rcall	send_eol
+	clz
+2:
+	ldi	r24, lo8(uart_rx)
+	ldi	r25, hi8(uart_rx)
+	mov	r22, r20
+	push	r20
+	breq 3f
+	rcall	parse_command
+3:	
+	pop	r24
+rx_dismiss:
+	cli
+	lds	r18, uart_rx_w
+	clr	r19
+	clr	r20
+	cpi	r18, 15
+	brcc 3f
+	sub	r18, r24
+	breq 3f
+	brcs 3f // TCNH, n > w
+	ldi	r26, lo8(uart_rx)
+	ldi	r27, hi8(uart_rx)
+	movw	r30, r26
+	add	r30, r24
+	adc	r31, r1
+1:	
+	ld	r25, Z+
+	st	X+, r25
+	subi	r19, -1
+	cpi	r25, '\n'
+	brne 2f
+	tst	r20
+	brne 2f
+	mov	r20, r19
+2:	
+	cp	r19, r18
+	brne	1b
+3:
+	sts	uart_rx_mes, r20
+	sts	uart_rx_w, r19
+	sei
+	ret