Forum Rule: Always post complete source code & details to reproduce any issue!
Results 1 to 22 of 22

Thread: serial_write_buffer_free return value calculation?

  1. #1
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    9,770

    serial_write_buffer_free return value calculation?

    opps. no problem here - move along
    Last edited by defragster; 09-15-2015 at 07:37 AM.

  2. #2
    Senior Member
    Join Date
    Jan 2015
    Location
    SF Bay Area
    Posts
    255
    I want to know why the free value returned is 1 less?
    say I set tx_buffer is 512 and empty, call to availableForWrite returns 511. Shouldn't it be 512?

  3. #3
    Senior Member
    Join Date
    Aug 2013
    Location
    Gothenburg, Sweden
    Posts
    293
    I havnt checked the code, but, it is common for circular buffers to have a capacity of one less than the number of entries.

    This is because the read and write pointer relations are exactly the same for a buffer with 0 elements in it, and a buffer with 512 elements in it.

  4. #4
    Senior Member
    Join Date
    Jan 2015
    Location
    SF Bay Area
    Posts
    255
    so does that mean I cannot really send 512 in one shot? it will send as 511 and 1 ?

    actually, the problem is, if buffer is set to 512, and I am waiting for 512 available before I write, it will never happen. It is not intuitive.

    Since the TX_BUFFER_SIZE is internal to the library, I think the method should return true or false instead to be more intuitive, as it seems as long as there is 1 byte available, you can call the write function.
    Last edited by doughboy; 09-15-2015 at 06:26 PM.

  5. #5
    Senior Member
    Join Date
    Jun 2013
    Location
    So. Calif
    Posts
    2,828
    Look around the 'web for ring buffer code.
    Most use pointers, not indices. And many use a counter rather than the difference of head and tail (modulo).

  6. #6
    Senior Member
    Join Date
    Aug 2013
    Location
    Gothenburg, Sweden
    Posts
    293
    The problem with the counter is that if we have one process writing to the buffer (main loop) and another reading from the buffer (serial irq handler) then the counter must be updated from both the writing and from the reading process creating a potential synchronization issue (I know standard teensy code doesnt use processes, but its a useful mental framework for main program context versus interrupt handler context). Using a write pointer only modified by the write process/context, and a read pointer only modified from the read process/context works stably without a lot of lock handling. This relies on the fact that on Cortex cores a 32 bit write is atomic.

  7. #7
    Senior Member
    Join Date
    Jan 2015
    Location
    SF Bay Area
    Posts
    255
    I see absolutely no reason why the serial_write_buffer_free function cannot return the actual free bytes. I think this is a bug.

  8. #8
    Senior Member
    Join Date
    Aug 2013
    Location
    Gothenburg, Sweden
    Posts
    293
    Bug or no bug, I wont argue, but you can see it this way:
    Out of the buffer bytes allocated one byte is reserved to keep track of full or empty status of the buffer.
    The remaining n-1 bytes are available for buffer data, and that number is reported.

    A similar situation exists for C character strings, where the last byte is reserved for a terminating 0 byte. Not respecting this will end you up in lots of fun debugging of weird problems.

  9. #9
    Senior Member
    Join Date
    Jun 2013
    Location
    So. Calif
    Posts
    2,828
    Quote Originally Posted by mlu View Post
    Bug or no bug, I wont argue, but you can see it this way:
    Out of the buffer bytes allocated one byte is reserved to keep track of full or empty status of the buffer.
    The remaining n-1 bytes are available for buffer data, and that number is reported.

    A similar situation exists for C character strings, where the last byte is reserved for a terminating 0 byte. Not respecting this will end you up in lots of fun debugging of weird problems.
    that defense doesn't resonate here!

  10. #10
    Senior Member
    Join Date
    Jan 2015
    Location
    SF Bay Area
    Posts
    255
    I understand your explanation why the -1.
    What I am saying there is absolutely no reason why -1 should be there. as stevech pointed out, there are plenty of implementations that use all bytes of the buffer. I used circular buffer and never had the need to lose 1 byte.
    The analogy is not comparing apples to apples. I would agree if there was no tail pointer, like c character string, you do not have length, so you use terminator for that. I'm not sure if you are saying all circular buffer must lose 1 byte and it can never use all bytes.

  11. #11
    Senior Member PaulStoffregen's Avatar
    Join Date
    Nov 2012
    Posts
    20,592
    Here's several replied, in one quick message (yeah, I'd fallen a little behind while wrapping up Teensy 3.2.... and there's still a *lot* of places on the site to update)

    Quote Originally Posted by mlu View Post
    it is common for circular buffers to have a capacity of one less than the number of entries.
    Yes, this is exactly what's happening here.

    Quote Originally Posted by doughboy View Post
    so does that mean I cannot really send 512 in one shot? it will send as 511 and 1 ?
    The simple answer is yes, it does indeed mean a 512 byte buffer only allows you to write 511 in a single shot. If you want to write 512 in a single shot, simply make the buffer 513. Easy, right?

    The complex answer involves realities of timing and how the actual underlying hardware really works. Serial1 and Serial2 have 8 byte FIFOs, and the UART holds 1 byte in its transmitter. The first interrupt happens very quickly, which drains 9 bytes from the buffer. So for only a very tiny extra delay, the effective buffer size becomes 511+9.

    Quote Originally Posted by doughboy View Post
    Since the TX_BUFFER_SIZE is internal to the library, I think the method should return true or false instead to be more intuitive, as it seems as long as there is 1 byte available, you can call the write function.
    A simple true/false return status would not allow you to send as much data as the buffer can currently take.

    If you only need a true/false, you can certainly just use the integer that way, since C++ treats zero as false and non-zero as true when you use an integer as a boolean.

    But there are plenty of applications that generate data as they go. For those, knowing the actual maximum amount the buffer can take is really useful. Restricting the info to only true/false would be a huge step backwards!


    Quote Originally Posted by stevech View Post
    Look around the 'web for ring buffer code.
    Most use pointers, not indices. And many use a counter rather than the difference of head and tail (modulo).
    Steve, as usual you have quite a knack for posting generic (non-Teensy) stuff. Do you think in cases like this you could at least provide a link to the specific code you're talking about?

    Quote Originally Posted by doughboy View Post
    I see absolutely no reason why the serial_write_buffer_free function cannot return the actual free bytes. I think this is a bug.
    It is returning the actual free bytes. That 1 extra byte isn't really free, because attempting to write to it would wait, due to the limitations of the head/tail index scheme.

    Perhaps a better implementation would truly use all the bytes. Maybe we'd even be talking about such an implementation, if Steve had gone to the trouble of linking to the ones he mentioned?!

    But 1 unused byte is a very small cost.... and from some of the other recent conversations, it's clear some people are indeed pushing Serial1 to 4.608 and even 6 Mbit/sec speeds. Efficiency matters! I must confess, I haven't done a lot of detailed timing analysis on baud rates over 1 Mbit/sec... but the current code seems to handle those speeds. The last thing I'd want to do is add changes to save 1 byte of RAM at the cost of handling very fast baud rates.

    Even if there is a fast way to use all the buffer's bytes, a major rewrite of the serial code is unlikely to happen anytime soon. And really, if you're increasing the buffer size, just increase it one more byte. How hard is that?

  12. #12
    Senior Member PaulStoffregen's Avatar
    Join Date
    Nov 2012
    Posts
    20,592
    Quote Originally Posted by doughboy View Post
    What I am saying there is absolutely no reason why -1 should be there ..... I used circular buffer and never had the need to lose 1 byte.
    Yet another comment about code, without the specific code!

    Most implementations do indeed lose 1 byte. Sometimes people are never aware of this, due to the realities of timing, where the first interrupt takes 1 or more bytes from the buffer very quickly. If you don't very carefully analyze & test the code, it's easy for it to appear to use all the bytes in the buffer.

    Then again, it's also possible to use all the bytes, using different coding. But how many more messages will we waste talking about such ways in abstract terms... without actually seeing the real source code?!

  13. #13
    Senior Member
    Join Date
    Jan 2015
    Location
    SF Bay Area
    Posts
    255
    I think the simple solution is for HardwareSerial code to allocate TX_BUFFER_SIZE+1.

  14. #14
    Senior Member
    Join Date
    Aug 2013
    Location
    Gothenburg, Sweden
    Posts
    293
    Quote Originally Posted by stevech View Post
    that defense doesn't resonate here!
    In what way??, its not a defense, its an explanation how the code works, there are many ways to skin a cat, but dont pretend this is not one of the standard and stable versions.

  15. #15
    Senior Member
    Join Date
    Aug 2013
    Location
    Gothenburg, Sweden
    Posts
    293
    If you really really need 512 bytes in the buffer, well go ahead and set TX_BUFFER_SIZE = 513, but I doubt many codes will ever note the difference, the amount of data in the transmit buffer keeps changing since the serial irq will keep sending characters out and changing the number of characters in the buffer.

  16. #16
    Senior Member
    Join Date
    Aug 2013
    Location
    Gothenburg, Sweden
    Posts
    293
    All circular bufferimplementations must not loose one byte, but as mentioned before, then there must be a counter keeping track of the number of bytes in the buffer. That will create huge problems when the buffer is accessed both from main loop and from interrupt code. If that is not needed, then sure its easy to make a circular buffer with a counter, and all bytes used, but it wont give you fast serial I/O. So its all really about defining your preferences. And in this case its not a bug, its a feature, and if you dont agree, no one stops you from implementing your own version.
    Last edited by mlu; 09-15-2015 at 10:09 PM.

  17. #17
    Senior Member PaulStoffregen's Avatar
    Join Date
    Nov 2012
    Posts
    20,592
    I'm willing to consider alternate implementations.

    I do hope everyone can understand my reluctance to substantially change code that's working very well. Over the last few years, that serial code has had several rounds of bug fixes and performance improvements, involving many hours of my dev time, based on conversations here on this forum. I'm *still* planning more improvements, like optimized readBytes() and preventing it from compiling into projects not using the serial ports (serialEvent and the ARM fault hander make that complicated).

    A major overhaul of how it manages the buffers isn't in the plans, and very likely would risk the stability and performance of the code.

    But of all the makers of Arduino compatible boards, I'm probably among the most willing to massively overhaul complex low-level code if there's a substantial improvement to be gained. Compare with others... even on Arduino's own boards, they spent years without any transmit buffering on any boards (while Teensy 2.0 had it). Even Arduino Due just got transmit buffering after nearly 3 years without, and Arduino Zero is still lacks any transmit buffering. Even on Due and AVR-based Arduino, they use this same scheme which wastes one byte of the buffer.

    I do care deeply about good buffering and high performance hardware serial on Teensy. I'm willing to do a *lot* of work to make even small improvements! I especially care about facilitating high speed baud rates and minimizing CPU usage so there's processing power left over to actually *do* something with such fast data streams.

    While I'm willing to consider other approaches, something I'm not willing to do is keep debating abstract descriptions of code. Proposals need to be expressed with actual source code (links to github repos are fine), or at the very least precise algorithms.
    Last edited by PaulStoffregen; 09-15-2015 at 11:09 PM.

  18. #18
    Senior Member
    Join Date
    Jun 2013
    Location
    So. Calif
    Posts
    2,828
    @Paul..... It's wise to look outside Arduino-land, minimize reinvention - there a many examples of ring buffer implementations that don't have that wasted byte design. In fact, most that I've seen don't.

    This is simply a comment on common software design. Check in with Mr. Knuth.

    If you wish, I can dig in my own archives and find some example code of a standard ring buffer implementation - a few of which I've written in the past.
    Last edited by stevech; 09-15-2015 at 11:38 PM.

  19. #19
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    9,770
    The 1 [or 'word' 4] byte RAM 'user' fix as noted would be to adjust the buffer upward.

    A fix with all buffer used - if the examples below are indicative(and good): empty could go from 'head==tail' to 'tail==RX_BUFFER_SIZE'? Every function would need touched

    Code:
    int serial_available(void)
    {
    	uint32_t head, tail;
    
    	head = rx_buffer_head;
    	tail = rx_buffer_tail;
    	if (head > tail) return head - tail;
    	if (RX_BUFFER_SIZE == tail) return 0;
    	return RX_BUFFER_SIZE + head - tail;
    }
    
    int serial_getchar(void)
    {
    	uint32_t head, tail;
    	int c;
    
    	head = rx_buffer_head;
    	tail = rx_buffer_tail;
    	// if (head == tail) return -1;
    	if (RX_BUFFER_SIZE == tail) return -1;
    	if (++tail >= RX_BUFFER_SIZE) tail = 0;
    	c = rx_buffer[tail];
    	if (++head >= RX_BUFFER_SIZE) head = 0;
    	if (head == tail) tail = RX_BUFFER_SIZE;
    	rx_buffer_tail = tail;
    	return c;
    }
    Last edited by defragster; 09-15-2015 at 11:47 PM.

  20. #20
    Senior Member
    Join Date
    Jun 2013
    Location
    So. Calif
    Posts
    2,828
    Pages 1-3 in the below are an on-line example of using a counter of bytes in the buffer. One must be careful with interrupts and volatile and so on.

    http://www.embedded.com/electronics-...he-ring-buffer

  21. #21
    Senior Member
    Join Date
    Jun 2013
    Location
    So. Calif
    Posts
    2,828
    An App note circa 2000 on a simple interrupt handler and ring buffer for UART tx,rx.
    Using a counter for each. Rather than comparing head-tail.

    Illustrating the commonplace technique rather than ready-to-eat code for Teensy.
    http://supp.iar.com/Support/?note=18523

  22. #22
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    9,770
    Wow - I missed a whole stream of 'stuff' when I revisited and saw Paul's request for concrete code examples . . .

    Those READ functions were only compiled in my head and just samples of the code I had open - from my (so far) failed attempt to get ReadBytes() to work. I assume the same process would suffice in the WRITE code. It will take added code for the checks as 'head==tail' more naturally "falls out" in the code - and small details will matter - like the vars used for head&tail jumping to dual byte a 'bit' early.

    Even 'this' change would be a significant revisit to the code and prone to errors and add to the code (size & complexity) and not make anything faster(other than that lost byte). However, any more drastic implementation with reference to yet_another_var would invite more work and open contention over ISR modified values. Currently writes should never change TAIL and reads should never change HEAD. Even this change would mean a write would have to check tail and set it right when an empty buffer gets data - but in that case the read/peek check should just fail 'harmlessly' (unless written too soon) as data just arrived and the 'race' to see the head .vs. tail value might have made it see 'empty' as well - and a read would never touch tail once set to rwX_BUFFER_SIZE.

    Code:
    #if RX_BUFFER_SIZE > 254
    static volatile uint16_t rx_buffer_head = 0;
    static volatile uint16_t rx_buffer_tail = RX_BUFFER_SIZE;
    #else
    static volatile uint8_t rx_buffer_head = 0;
    static volatile uint8_t rx_buffer_tail = RX_BUFFER_SIZE;
    #endif
    <edit>*re-reading - since I did read() examples- when I say write(s) I mean a 'buffer filler', given the ISR writes to the read buffer and another ISR reads from the write buffer after the sketch writes - no chance of confusion there.
    Last edited by defragster; 09-16-2015 at 10:55 PM.

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •