Skip to content

Harden Print::printf() method #795

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 27, 2019
Merged

Conversation

fpistm
Copy link
Member

@fpistm fpistm commented Nov 26, 2019

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
Copy link
Contributor

MCUdude commented Nov 26, 2019

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
Copy link
Member Author

fpistm commented Nov 26, 2019

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

@MCUdude
Copy link
Contributor

MCUdude commented Nov 26, 2019

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

@fpistm
Copy link
Member Author

fpistm commented Nov 26, 2019

This is possible and tested.

@fpistm
Copy link
Member Author

fpistm commented Nov 26, 2019

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
Copy link
Contributor

MCUdude commented Nov 26, 2019

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
Copy link
Contributor

MCUdude commented Nov 27, 2019

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