Skip to content

Harden Print::printf() method#795

Merged
fpistm merged 1 commit into
stm32duino:masterfrom
fpistm:printf_hardening
Nov 27, 2019
Merged

Harden Print::printf() method#795
fpistm merged 1 commit into
stm32duino:masterfrom
fpistm:printf_hardening

Conversation

@fpistm

@fpistm fpistm commented Nov 26, 2019

Copy link
Copy Markdown
Member

Based on @PaulStoffregen works for Teensyduino:
https://github.com/PaulStoffregen/cores

  • Remove PRINTF_BUFFER usage.
  • Aligned declaration with printf() one by returning the number of characters printed (excluding the null byte used to end output to strings).

Tested:

  • core debug
  • standard printf usage on the DEBUG_UART
  • Serial over USB

Harden #780 from @MCUdude.

@fpistm fpistm added the enhancement New feature or request label Nov 26, 2019
@fpistm fpistm added this to the 1.8.0🎄 🎅 milestone Nov 26, 2019
@fpistm fpistm self-assigned this Nov 26, 2019
@MCUdude

MCUdude commented Nov 26, 2019

Copy link
Copy Markdown
Contributor

Can you verify that this can be used on two or more serial ports at the same time? I tested out the version from the Teensy core, but Serial.printf() and Serial1.printf() all printed to the same serial port. Adafruits implementation (found in #780) worked fine.

@fpistm

fpistm commented Nov 26, 2019

Copy link
Copy Markdown
Member Author

Serial is just an aliases to a SerialX instance. That why it print on the same serial port.

@MCUdude

MCUdude commented Nov 26, 2019

Copy link
Copy Markdown
Contributor

It's still important that we can use SerialX.printf on more than one serial port at the time. Will this be possible?

@fpistm

fpistm commented Nov 26, 2019

Copy link
Copy Markdown
Member Author

This is possible and tested.

@fpistm

fpistm commented Nov 26, 2019

Copy link
Copy Markdown
Member Author

Example for Nucleo-F411
Serial1 uses PA10 and PA9
Serial is mapped on Serial2 connected to STLink

HardwareSerial Serial1(USART1);

const char * lorem = "Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged. It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum passages, and more recently with desktop publishing software like Aldus PageMaker including versions of Lorem Ipsum.";

void setup() {
  Serial.begin(9600);
  Serial1.begin(9600);
}

void loop() {
  static uint32_t i = 0;
  Serial.printf("Iter %i --> %s\n", i++, lorem);
  Serial1.printf("Iter %i --> %s\n", i++, lorem);
}

@MCUdude

MCUdude commented Nov 26, 2019

Copy link
Copy Markdown
Contributor

I did test the Teensy implementation on a F401Rx board. I manually defined Serial1 at the beginning at the sketch and looked at the output in the serial monitor (Serial) and with a logic analyzer (Serial1). Even though it shouldn't, I got the same output in the serial monitor and in the logic analyzer capture window. However, I can test this particular implementation on by Nucle F401RE board tomorrow.

@MCUdude

MCUdude commented Nov 27, 2019

Copy link
Copy Markdown
Contributor

I just tested it on my Nucleo F401RE. Works perfectly! I do agree that this implementation is better than #780. And getting rid of the buffer is definitely a win!

based on @PaulStoffregen works for Teensyduino:
https://github.com/PaulStoffregen/cores

Remove PRINTF_BUFFER usage.
Aligned declaration with `printf()` one by returning the number
of characters printed (excluding the null byte used to end output
to strings).

Signed-off-by: Frederic Pillon <frederic.pillon@st.com>
@fpistm fpistm merged commit 3918800 into stm32duino:master Nov 27, 2019
@fpistm fpistm deleted the printf_hardening branch November 27, 2019 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants