PR #184 - STM32F1 USART Improvements

What could be included in further releases, or for the forum.
User avatar
mrburnette
Posts: 1783
Joined: Mon Apr 27, 2015 12:50 pm
Location: Greater Atlanta
Contact:

Re: PR #184 - STM32F1 USART Improvements

Post by mrburnette » Tue Jul 12, 2016 4:33 pm

FYI - Only: Code bloat
Sketch is a modified sketch rewritten to use USB serial to display all the Maple Mini's Ax values.

Original STM32F1 code:
Sketch uses 14,148 bytes (11%) of program storage space. Maximum is 122,880 bytes.
Global variables use 2,560 bytes of dynamic memory.
Overlaying the contents of Buffered_InterruptBased_USART_TX__OK.ZIP into my existing STM32F1 directory and recompiling.
New STM32F1 serial interrupt changes:
Sketch uses 14,604 bytes (11%) of program storage space. Maximum is 122,880 bytes.
Global variables use 2,808 bytes of dynamic memory.
Observation:
Other than code-bloat for a USB-only sketch, the output of the Maple Mini over the USB-serial worked well. No issues, errors, warnings.

Sketch:

Code: Select all

/*
  hacked by Ray Burnette to loop through all analog values
  Board view: http://letsmakerobots.com/files/maplemini2.jpg  
  Arduino 1.7.8 on Linux Mint 17.3 fully patched
      Sketch uses 13,948 bytes (12%) of program storage space. Maximum is 110,592 bytes.
      Global variables use 2,560 bytes of dynamic memory.
*/

int aValue ;
int settlingmS = 5000;

void setup() {
    // Configure the ADC pins
    for (int x =3; x < 12; x++) {
      pinMode(x, INPUT_ANALOG);
    }
  delay(settlingmS);
}

void loop() {
  for ( int analogInPin = 3; analogInPin < 12; analogInPin++)
  {
    // read the analog in value:
    aValue = analogRead(analogInPin);
    // print the results to the serial monitor:
    Serial.print("Analog pin#");
    Serial.print(analogInPin);
    Serial.print("\t = ");
    Serial.println(aValue);
  }
  delay(settlingmS);
}


edogaldo
Posts: 250
Joined: Fri Jun 03, 2016 8:19 am

Re: PR #184 - STM32F1 USART Improvements

Post by edogaldo » Tue Jul 12, 2016 4:57 pm

Overlaying the contents of Buffered_InterruptBased_USART_TX__OK.ZIP into my existing STM32F1 directory and recompiling
Well, this version uses static TX buffer implementation (same as RX buffer), I guess this explains the memory bloating..

[EDIT] Then there is interrupt handler extension and few more code lines to init the new buffer and use it which would explain the code bloating.

User avatar
Rick Kimball
Posts: 852
Joined: Tue Apr 28, 2015 1:26 am
Location: Eastern NC, US
Contact:

Re: PR #184 - STM32F1 USART Improvements

Post by Rick Kimball » Tue Jul 12, 2016 9:41 pm

edogaldo wrote:
mrburnette wrote: Also about dynamic buffering, as already said in another thread, I started working on a minimal solution for buffered interrupt-based TX which I posted here: viewtopic.php?f=3&t=1189&p=15297#p15297
(note this implementation probably still needs some change)
I like the idea of this one as a first step. I gave it a whirl and a few things seem to need work. I created a sketch to run the uart port at 2400 baud and would have expected lower time for doing a println of 80 characters than I got. Did you do any performance testing on that version? To see how much better it actually is?

-rick
-rick

edogaldo
Posts: 250
Joined: Fri Jun 03, 2016 8:19 am

Re: PR #184 - STM32F1 USART Improvements

Post by edogaldo » Wed Jul 13, 2016 4:53 am

Rick Kimball wrote: I like the idea of this one as a first step. I gave it a whirl and a few things seem to need work. I created a sketch to run the uart port at 2400 baud and would have expected lower time for doing a println of 80 characters than I got. Did you do any performance testing on that version? To see how much better it actually is?

-rick
Here's @simonf post which benchmarked the current TX implementation (polling) against the different baud rates: viewtopic.php?f=3&t=1189#p15004
You can use the same sketch to compare results.

I also added to Github this version: https://github.com/edogaldo/Arduino_STM ... TX-buffer)
(note: the last ")" should be part of the link..)

which should be, imo, the minimal cleanest implementation of buffered interrupt-based TX (using static buffering); anyway I couldn't still test it

[Edit] I could successfully test it.


Best, E.

User avatar
Rick Kimball
Posts: 852
Joined: Tue Apr 28, 2015 1:26 am
Location: Eastern NC, US
Contact:

Re: PR #184 - STM32F1 USART Improvements

Post by Rick Kimball » Thu Jul 14, 2016 3:03 pm

* correct link with the missing paren ')' ...

https://github.com/edogaldo/Arduino_STM ... TX-buffer)

-rick
-rick

User avatar
Rick Kimball
Posts: 852
Joined: Tue Apr 28, 2015 1:26 am
Location: Eastern NC, US
Contact:

Re: PR #184 - STM32F1 USART Improvements

Post by Rick Kimball » Thu Jul 14, 2016 3:12 pm

That branch seems to have minimal changes. I like it. However, this version also has the problem with Hardware::end() and not flushing the buffer.
https://github.com/edogaldo/Arduino_STM ... sart.c#L69

I've not extensively tested this code. However, just doing a visual inspection it seems like there might be race conditions. For example, this code looks like their might be a race between the the irq handler and setting of the interrupt enable flag:
https://github.com/edogaldo/Arduino_STM ... sart.c#L69

Have you thought about using atomic operations to safeguard against this?

-rick
-rick

edogaldo
Posts: 250
Joined: Fri Jun 03, 2016 8:19 am

Re: PR #184 - STM32F1 USART Improvements

Post by edogaldo » Thu Jul 14, 2016 5:28 pm

First of all thanks for fixing the link, I don't know why, the last ")" was left outside the link in my post (I tried to include it but unsuccessfully..)
Rick Kimball wrote:That branch seems to have minimal changes. I like it. However, this version also has the problem with Hardware::end() and not flushing the buffer.
https://github.com/edogaldo/Arduino_STM ... sart.c#L69
About the HardwareSerial.end() problem: I assume you are referring to your comment of not waiting for the TX transmission to be completed before disabling the USART, right, fixed it!

About "flushing the buffer": do you mean in the flush()?
Based on the current flush() implementation, a change could be add "usart_reset_tx(this->usart_device);"
Rick Kimball wrote: I've not extensively tested this code. However, just doing a visual inspection it seems like there might be race conditions. For example, this code looks like their might be a race between the the irq handler and setting of the interrupt enable flag:
https://github.com/edogaldo/Arduino_STM ... sart.c#L69

Have you thought about using atomic operations to safeguard against this?

-rick
Maybe there could be in the RX flow but it's as-is. I didn't make any change to RX and it seems to me there have never been any complains of race conditions in that code.

About TX, I'm pretty sure there are none (also in my tests I didn't observe any) anyway if you see any potential race condition please provide details.

About "atomic operations": yes, it could be good but it's above my current knowledge, and also I didn't see any atomic operation for buffer access in any implementation I investigated. Anyway, if you have any suggestion, we can discuss it here.


Best, E.
Last edited by edogaldo on Thu Jul 14, 2016 5:44 pm, edited 1 time in total.

danSTM
Posts: 6
Joined: Wed Mar 23, 2016 2:29 am

Re: PR #184 - STM32F1 USART Improvements

Post by danSTM » Tue Aug 09, 2016 7:44 pm

@edogaldo

Thanks for the work on this! I originally posted here: Blue Pill Serial.print slow, UNO faster? -> viewtopic.php?f=3&t=1189

I put my pills aside because of this, now I can use them. I wonder how many people using serial STM Arduino projects think they're getting the most out of the processor until they hit the serialTX speed bump?

I tried the SerialTXBuffer branch at https://github.com/rogerclarkmelbourne/Arduino_STM32 In my tests, I'm now getting the expected ~4.5x TX improvement over the UNO. :D

Certainly something like this needs to be added to the core! ...thanks again.

User avatar
RogerClark
Posts: 5923
Joined: Mon Apr 27, 2015 10:36 am
Location: Melbourne, Australia
Contact:

Re: PR #184 - STM32F1 USART Improvements

Post by RogerClark » Tue Aug 09, 2016 9:49 pm

guys

I will merge this at the weekend, as I think enough people have had time to test and comment on it

Post Reply

Who is online

Users browsing this forum: No registered users and 1 guest