ARMmbed / mbed-os

Arm Mbed OS is a platform operating system designed for the internet of things
https://mbed.com
Other
4.67k stars 2.98k forks source link

Mbed error output is not working on ARDUINO_NANO33BLE #12053

Closed asprenger closed 3 years ago

asprenger commented 4 years ago

Description of defect

The MBED_ASSERT() macro on ARDUINO_NANO33BLE halts the system (LED is blinking) but does not produce any console output. I'm testing with a debug build of this code:

#include "mbed.h"
#include "USBSerial.h"

USBSerial serial(false);

// Redirect FileHandles to get printf() working
FileHandle *mbed::mbed_override_console(int) {
  return &serial;
}

int i = 0;
int main() {
  serial.connect();  
  serial.wait_ready();
  printf("Trigger MBED_ASSERT\n\r");
  MBED_ASSERT(false);
}

The output I would expect is created in print_error_report() in mbed_error.c.

Looking into the code this is the sequence of functions called:

  1. mbed_assert.h: MBED_ASSERT()
  2. mbed_assert.c: mbed_assert_internal()
  3. mbed_error.c: mbed_error()
  4. mbed_error.c: print_error_report()
  5. mbed_board.c: mbed_error_printf()
  6. mbed_board.c: mbed_error_puts()
  7. mbed_retarget.c: write(STDERR_FILENO,...)

From this I checked if mbed_error_printf() is working with the following code:

#include "mbed.h"
#include "USBSerial.h"

USBSerial serial(false);

// Redirect FileHandles to get printf() working
FileHandle *mbed::mbed_override_console(int) {
  return &serial;
}

int i = 0;
int main() {
  serial.connect();  
  serial.wait_ready();

  for (int i=0; i<5; i++) {
    printf("BEFORE\n\r");   
    ThisThread::sleep_for(500);
  }  
  mbed_error_printf("SYSTEM ERROR");
  for (int i=0; i<5; i++) {
    printf("AFTER\n\r");    
    ThisThread::sleep_for(500);
  }
}

This code prints "BEFORE" 5 times and than the system halts with the LED blinking. So there is some problem in the mbed_error_printf(). I verified this with a JLINK debugger but I'm getting stuck in some USB device code that I have not understood yet.

Target(s) affected by this defect ?

ARDUINO_NANO33BLE

Toolchain(s) (name and version) displaying this defect ?

GCC_ARM gcc version 8.3.1 20190703 (release) [gcc-8-branch revision 273027] (GNU Tools for Arm Embedded Processors 8-2019-q3-update)

What version of Mbed-os are you using (tag or sha) ?

https://github.com/ARMmbed/mbed-os/#cf4f12a123c05fcae83fc56d76442015cb8a39e9

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

mbed-cli version 1.10.1

How is this defect reproduced ?

See scripts included above.

0xc0170 commented 4 years ago

cc @facchinm

asprenger commented 4 years ago

I'm running into another issue with the console output. When I run the following code the execution just stops after about 1000 iterations:

#include "mbed.h"
#include "USBSerial.h"

USBSerial serial(false);

// Redirect FileHandles to get printf() working
FileHandle *mbed::mbed_override_console(int) {
  return &serial;
}

int i = 0;
int main() {
  serial.connect();  
  while(1) {
    printf("Some text (%d)\n\r", i++);
  ThisThread::sleep_for(500);
  }
}

This code should just work. I wonder if I'm using the USBSerial class correctly or if there is another way to write to a serial terminal. I did not experience this problems using Arduino code. Arduino also has its own implementation of USBSerial (in an arduino namespace) https://github.com/arduino/ArduinoCore-nRF528x-mbedos/blob/master/cores/arduino/USB/PluggableUSBSerial.h

kjbracey commented 4 years ago

For fault handling to work, the FileHandle device used for the console override must be tolerant to being called from exception handler context, at least for write.

DirectSerial does this by being extremely simple. UARTSerial does this by having a special-case in its write - redirects to write_unbuffered.

It's possible that USBSerial would need such a special case added.

ciarmcom commented 4 years ago

Thank you for raising this detailed GitHub issue. I am now notifying our internal issue triagers. Internal Jira reference: https://jira.arm.com/browse/IOTOSM-2269

ciarmcom commented 3 years ago

We closed this issue because it has been inactive for quite some time and we believe it to be low priority. If you think that the priority should be higher, then please reopen with your justification for increasing the priority.