compile error with -D SERIAL_RX_BUFFER_SIZE=256

[ieb]

I get the following compile error[1] when I try and build with -D SERIAL_RX_BUFFER_SIZE=256, which is fixed by https://github.com/ieb/megaTinyCore/commit/7cfb0370abf3b438c09f1f91ae23a79ce4821397 ... but, is that the right way or should I be using some other define as well ?

Why the huge buffer ? I have some compute that takes 63ms every 60s, which is hard to interleave with receive (world magnetic field model calculations).

The patch works and appears to be stable on an Attiny3224 running at 19200 baud with a stream of data from a ublox gnss module that compiles into [1] with the buffer. The stream is almost continuous, hence the overflows.

RAM: [==== ] 36.7% (used 1126 bytes from 3072 bytes) Flash: [========= ] 87.6% (used 28713 bytes from 32768 bytes)

1 Compiling .pio/build/attiny3224/FrameworkArduino/UART0.cpp.o Compiling .pio/build/attiny3224/FrameworkArduino/UART1.cpp.o /Users/ieb/.platformio/packages/framework-arduino-megaavr-megatinycore@2.6.8/cores/megatinycore/UART1.cpp: In function 'void vector_26()': /Users/ieb/.platformio/packages/framework-arduino-megaavr-megatinycore@2.6.8/cores/megatinycore/UART1.cpp:60:23: error: '_rx_complete_irq' is not a member of 'HardwareSerial' HardwareSerial::_rx_complete_irq(Serial1); ^~~~ /Users/ieb/.platformio/packages/framework-arduino-megaavr-megatinycore@2.6.8/cores/megatinycore/UART0.cpp: In function 'void __vector_17()': /Users/ieb/.platformio/packages/framework-arduino-megaavr-megatinycore@2.6.8/cores/megatinycore/UART0.cpp:87:23: error: '_rx_complete_irq' is not a member of 'HardwareSerial' HardwareSerial::_rx_complete_irq(Serial); ^~~~ /Users/ieb/.platformio/packages/framework-arduino-megaavr-megatinycore@2.6.8/cores/megatinycore/UART1.cpp: In function 'void vector_27()': /Users/ieb/.platformio/packages/framework-arduino-megaavr-megatinycore@2.6.8/cores/megatinycore/UART1.cpp:83:23: error: '_tx_data_empty_irq' is not a member of 'HardwareSerial' HardwareSerial::_tx_data_empty_irq(Serial1); ^~~~~~ /Users/ieb/.platformio/packages/framework-arduino-megaavr-megatinycore@2.6.8/cores/megatinycore/UART0.cpp: In function 'void __vector_18()': /Users/ieb/.platformio/packages/framework-arduino-megaavr-megatinycore@2.6.8/cores/megatinycore/UART0.cpp:110:23: error: '_tx_data_empty_irq' is not a member of 'HardwareSerial' HardwareSerial::_tx_data_empty_irq(Serial); ^~~~~~ [.pio/build/attiny3224/FrameworkArduino/UART0.cpp.o] Error 1 [.pio/build/attiny3224/FrameworkArduino/UART1.cpp.o] Error 1

[SpenceKonde]

Goddamnit! STILL echoes of that serial ISR overhaul. The change that caused this was first in 2.5.x (maybe it was busted so badly in 2.5.0 that "Was the bug present in that version" wasn't meaningful) It has led to about half of the emergecncy bugfix releases since then,, and combined with the attachInterrupt change, those have made up a large majority of recent updates,)

looks like I am using a different test in the conditional compilation in different places. The inline asm is only written for cases where the buffer is a power of 2 not larger than 256, because if you are using a buffer size that isn't a power of two you obviously don't care about performance. 256b buffers also come at a performance penalty that hurts people who are operating at very high baud rates, but you're running at quite a low baud, so that wont be noticable for you (it's like a clock cycle or two per byte or something. Matters a lot at max baud rate, because our ISR comes in with basically not a cycle to spare if it is to receive data continuously at baud rate = F_CPU/8. I reckon in one place it's using a test that doesn't the case where the buffer size is 256 which was added after the initial serial changes.

I will note that this is not considered to be a high priority as it cannot happen in supported configurations (supported configuration means any configuration that can be generated by the Arduino IDE based on the option selections in tools submenus as defined in boards.txt). Since no menu is provided for setting the serial buffer, and it is only possible to change the size of it by modifying either the board definitions (which is effectively what you are doing by passing the -D parameter) or the UART.h file, and which is not even an "advertised" feature of the core

Oh., and I think 128 byte buffers should work assuming the USART is doing 8 data bits and 2 framing bits, and assuming you're speaking truth when you claim it's 63ms for the calculations. 19200 baud should give you 0.5208 ms between characters but you are very very close to the wire at 128. Counting the two byte fifo you would have 130 bytes of buffering enough for 67.7 ms of interrupted processing. So at 63 ms and 128 byte buffers, it should work. but if you were off by 5% on the duration of the calculations after the ISRs have punched holes in the execution, slightly prolonging it, then you couldn't do it with 128b rx buffer.

Platform IO generates so damned many inquiries, most of them people requesting docs and files that I don't know what they are or how to generate them, and I don't have time to learn an IDE just so I can write documentation for it (that's how terrible docs are written anyway - nobody can write good docs about something they don't use or have any interest in.... )When I don't have time to keep up with things currently https://github.com/SpenceKonde/megaTinyCore/issues/978 https://github.com/SpenceKonde/DxCore/issues/319

If someone were to address some other pio issue with a PR, I might be able to get this issue fixed in a more timely manner ;-)

[SpenceKonde]

Yeah, looks like the change to permit 256b buffers (which comments indicate I had doubts would work correctly) was ported from DxCore to UART.cpp, but not UART0.cpp and UART1.cpp.

The way serial works in practice is almost painful to look at. Because almost universally, what the code outside of the interrupt does with what it receives.... is to take it out of that buffer and put it in another buffer! So we have an ISR that takes it out of the hardware fifo buffer (2 bytes) and puts it into a software ring buffer buffer then non-ISR code that copies those to another buffer until it gets enough for a command.

Now, like I said based on the comment I put in, I am not certain that it works right (and if it doesn't, the fix might (will) be a BFD)

[ieb]

I shared how I use your git repo directly on one of those issues, perhaps it will help others. Thanks for the pointers indicating non asm code is fine given the super low baud rate.

I've had a device with 256b and 19200 running now for 96h on my desk with no lost data from a UBlox GNSS module inbound on the UART. The 63ms for compute is ontop of sending CAN over SPI to a MCP2515 which might be why 128b was not quite enough. If I can sacrifice a bit more ram I can split up the compute and should be able to go back to a 128b buffer or less. The calculations are in 2 nested loops over 12, so it looks quite possible. (https://github.com/nhz2/XYZgeomag/blob/master/src/XYZgeomag.hpp#L185)