Split debug menu and add Segger RTT as option for logging output #423

henrygab · 2020-01-16T23:00:26Z

[EDIT: Please see new PR #431.]

Resolves #416

At its core, this PR does three things, but does _ not_ change the default build behavior (CFG_DEBUG=0).

Separates the concept of CFG_DEBUG (debug level in UI) from whether or not SysView is enabled. As noted in Split debug menu and add Segger RTT as option for logging output #416, only enabling SysView with CFG_DEBUG set to 3+ can prevent SysView from being used to find Heisenbugs. Now, SysView can be enabled even when CFG_DEBUG is set to zero.
Exposes a "default" logger that derives from the Stream class. This default logger is used for all built-in board logging output, and is exposed for use by libraries if they choose to use it. By default, this is still Serial, and thus has no appreciable effect when using default build configuration.
Add support for Segger RTT as the default data log, helping to track Heisenbugs that otherwise would be hidden when logging levels are increased. Serial remains the default, and a new menu option is provided to select the use of RTT for board-level logging.

Tested on nRF52840 Feather Express hardware with various option combinations, and RTT tested both with and without JLink attached.

NOTE:

Serial.begin() is no-op. Why does main.cpp call it (around line 50)?
[edit: not a no-op on nrf52832, which uses hardware uart]

[edit Also tested building using arduino-builder.exe … made life so much easier!]

1. separate debug level from systemview enable/disable This allows enabling systemview without also setting really high debug level. High debug level caused lots of serial output, which slowed things down and added Heisenbugs. 2. Add option to redirect output via Segger RTT instead of serial output. Updates to Segger RTT v6.60d. Boards.txt / platform.txt are used to allow UI to show the new options, which causes the following to be defined during build: * CFG_DEBUG := As before, from 0 (disabled) to 3 (very verbose) * CFG_LOGGER := Where to output logs / printf; either 1 (serial) or 2 (RTT) * CFG_SYSVIEW := Enables SysView APIs if set to non-zero value

Add additional functions to Segger_RTT_Stream_t, so callers do not need to change code if they are expecting a serial port. Per Arduino web page, serialEvent() is not supported on all boards. RTT also has no interrupt-based notification when the host writes data, so for now, not implementing a serialEvent workaround (e.g., timer or loop-based manual check for available data).

This function is identical to SEGGER_RTT_TerminalOut(), except that it sends the caller-specified arbitrary bytes to the terminal. This makes it much easier to use the multiple terminals as a replacement for STDERR, STDOUT, etc., because the function handles all the edges cases, and just needs the formatted string to be provided.

Added distinct header and implementation files. Also redirected _write() to RTT when logging to RTT.

If RTT mode is set to `SEGGER_RTT_MODE_BLOCK_IF_FIFO_FULL`, then the program will halt when the buffer fills unless a JLink is connected and draining the buffer. Fix by setting to `SEGGER_RTT_MODE_NO_BLOCK_SKIP`. Always define `Adalog_Default_Logger`, even when CFG_DEBUG is not defined.

In case later have a logger that requires begin(). Also makes conversion from Serial easier, as don't need to special-case calls to begin().

Compile-time assertions are awesome, whether via static_assert() or #if blocks, as they can show when assumptions are false, and they catch new changes that make prior assumptions no longer true.

for Adalog_Default_Logger.

hathach · 2020-01-31T03:48:09Z

Thanks @henrygab your reason is all valid, however, I don't prefer to have 2 extra menu options just to deal with this. Especially there is only 5% of people will ever use it. This Arduino repo still primarily target beginner, most don't ever have a jlink to begin with.

To sum up, I think we should still keep the menu as it is with a simple modification to automatically enabled CFG_SYSVIEW when CFG_DEBUG=3, else you could just modify the source to manually enable CFG_SYSVIEW alone while keeping debug level = 0. Same with logger with SYSVIEW, please add CFG_SYSVIEW_LOG somewhere in the source where you could just toggle 0/1 when needed.

I am really like your work, unfortunately, this PR will be only used by a very small users and could cause confusion to the vast majority.

henrygab · 2020-01-31T06:30:29Z

Hi @hathach , and welcome back. I hope your time off was excellent.

Based on your review, I am removing the menu options as requested. I also updated .gitignore to ignore platform.local.txt and boards.local.txt, as they should never be checked in (at least to master).

In this way, if desiring to use the Segger RTT and/or Segger SysView, it is now easily done using either platform.local.txt (for always changed) or boards.local.txt (for menu-options in Arduino UI).

hathach

Please revert the original RTT sources files, we will only use it as it is.

boards.txt

cores/nRF5/freertos/config/FreeRTOSConfig.h

cores/nRF5/sysview/SEGGER/SEGGER_RTT.c

cores/nRF5/sysview/SEGGER/SEGGER_RTT.h

hathach · 2020-01-31T09:46:22Z

Hi @hathach , and welcome back. I hope your time off was excellent.

Thank you very much, it is indeed excellent, except for the Coronavirus ~ . ~

Based on your review, I am removing the menu options as requested. I also updated .gitignore to ignore platform.local.txt and boards.local.txt, as they should never be checked in (at least to master).

In this way, if desiring to use the Segger RTT and/or Segger SysView, it is now easily done using either platform.local.txt (for always changed) or boards.local.txt (for menu-options in Arduino UI).

I don't really like this approach either, but it is doable. I would expect advanced user modify the source file directly without the help of IDE e.g

#if CFG_DEBUG > 3
#define CFG_SEGGER    1
#endif

#ifndef  CFG_SEGGER
#define CFG_SEGGER    0 // change to 1 to use with SYSVIEW without logging
#endif

henrygab · 2020-01-31T11:21:27Z

I will have to think about how to meet all the changes you would like.
Some things are easy. Some less so.

hathach

Sorry for not being clear. I mean we should leave the Segger RTT source code untouched.

cores/nRF5/sysview/SEGGER/SEGGER_RTT.h

Per Hathach request, using the files provided under SysView rather than provided with JLink.

Do not use Assembly version.

Hathach requested not to modify any SEGGER-provided files. However, it is not possible to avoid some type of modification. This is the closest without some type of modification.

This hack is brought to you by letters 'Q' and 'Z', and by the numbers '0' and '1'.

henrygab · 2020-02-01T06:29:26Z

Please see new PR #431.

It is a squashed merge from this branch, and thus will provide a cleaner history.

henrygab added 9 commits January 13, 2020 10:54

Add Peek() to SEGGER RTT.

75145eb

Expose default Stream for logging.

f67a5d5

Add RTT Serial and Terminal class.

8d89c8e

Added distinct header and implementation files. Also redirected _write() to RTT when logging to RTT.

Use default logger for Bluefruit52Lib library.

4bb3e65

Fix build errors.

b7939d6

henrygab requested review from hathach and ladyada and removed request for ladyada January 17, 2020 20:06

henrygab requested a review from ladyada January 25, 2020 18:12

henrygab added 3 commits January 25, 2020 10:24

Expose Adalogger underlying class

dade31e

In case later have a logger that requires begin(). Also makes conversion from Serial easier, as don't need to special-case calls to begin().

Fix non-native USB boards.

edb9f85

Compile-time assertions are awesome, whether via static_assert() or #if blocks, as they can show when assumptions are false, and they catch new changes that make prior assumptions no longer true.

Revert to exposing base Stream class only

5fe8c06

for Adalog_Default_Logger.

henrygab added 3 commits January 30, 2020 22:04

Remove/revert menu options

969ac52

Ignore boards.local.txt and platform.local.txt

666d72c

Merge branch 'master' into Segger_RTT

4981f73

hathach requested changes Jan 31, 2020

View reviewed changes

henrygab added 2 commits January 31, 2020 02:58

Remove accidental copy/paste

45cf21f

Remove outdated comment

897a56e

hathach reviewed Jan 31, 2020

View reviewed changes

cores/nRF5/sysview/SEGGER/SEGGER_RTT.h Outdated Show resolved Hide resolved

henrygab added 3 commits January 31, 2020 15:12

Unmodified SysView target source files

671a661

Per Hathach request, using the files provided under SysView rather than provided with JLink.

Unmodified, except when previously modified.

7f43816

Do not use Assembly version.

Fails to compile.

58156b2

Hathach requested not to modify any SEGGER-provided files. However, it is not possible to avoid some type of modification. This is the closest without some type of modification.

henrygab added 2 commits January 31, 2020 16:48

Compiles w/o modification to original SEGGER files

62dbfd4

This hack is brought to you by letters 'Q' and 'Z', and by the numbers '0' and '1'.

Remember to use extern "C" {}

1aaa510

henrygab closed this Feb 1, 2020

henrygab deleted the Segger_RTT branch March 11, 2020 22:30

henrygab mentioned this pull request Apr 23, 2020

use SWO & Segger RTT for logging. hathach/tinyusb#367

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Split debug menu and add Segger RTT as option for logging output #423

Split debug menu and add Segger RTT as option for logging output #423

Uh oh!

henrygab commented Jan 16, 2020 •

edited

Loading

Uh oh!

hathach commented Jan 31, 2020

Uh oh!

henrygab commented Jan 31, 2020

Uh oh!

hathach left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hathach commented Jan 31, 2020

Uh oh!

henrygab commented Jan 31, 2020

Uh oh!

hathach left a comment

Uh oh!

Uh oh!

henrygab commented Feb 1, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Split debug menu and add Segger RTT as option for logging output #423

Split debug menu and add Segger RTT as option for logging output #423

Uh oh!

Conversation

henrygab commented Jan 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hathach commented Jan 31, 2020

Uh oh!

henrygab commented Jan 31, 2020

Uh oh!

hathach left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hathach commented Jan 31, 2020

Uh oh!

henrygab commented Jan 31, 2020

Uh oh!

hathach left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

henrygab commented Feb 1, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

henrygab commented Jan 16, 2020 •

edited

Loading