Skip to content
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

Add support for SEGGER RTT STDIO #775

Closed
wants to merge 3 commits into from

Conversation

anaoum
Copy link

@anaoum anaoum commented Mar 30, 2022

Fixes #774

This PR adds support for using SEGGER RTT as an STDIO driver.

The code for the stdio_rtt implementation is quite simple and copies the structure of the other STDIO drivers. RTT support can be enabled in the same way as the other STDIO drivers, simply add pico_enable_stdio_rtt(${TARGET_NAME} 1) to your CMakeLists.txt and call stdio_init_all() in your initialisation code.

The official SEGGER_RTT code is included in the lib directory (the latest version, V762c). I have copied all files exactly as-is, despite many of the files being unnecessary/code samples/etc. I did this to make updates to newer versions more simple (just overwrite the entire directory with the latest version). Unfortunately SEGGER does not distribute this code as a git repo which could have been included as a submodule.

Happy to make improvements/adjustments to this implementation based on feedback.

@smurfix
Copy link

smurfix commented May 4, 2022

Why don't you re-package the Segger code as a git repository yourself (or clone (and update?) https://github.com/PromyLOPh/rtt)?

Did you ask them to please publish their code to github?

@anaoum
Copy link
Author

anaoum commented May 4, 2022

I find the problem with third parties hosting a clone of the official code is that they become out of date very quickly. To be fair, this problem would be the same by keeping the code copy in the pico-sdk, so I'm not sure what the best solution is...

The version you linked is 5 years old. I found another repo that seems to do a better job of keeping the code up to date: https://github.com/adfernandes/segger-rtt. I am happy to update my PR to use this third-party repo, but I would also like a maintainer of this project to chime in and advise what approach would most likely get merged.

I emailed SEGGER this morning to see if they would consider publicly hosting the code on a git repo.

@PhilippHaefele
Copy link

I was just searching for something else, but i think they already have the RTT source code on GitHub. Didn't checked if the version is the newest one (they always change the header on a release, but code often doesn't have any changes)

https://github.com/SEGGERMicro/RTT

Hope i could help a little bit

@adfernandes
Copy link
Contributor

adfernandes commented Jul 28, 2022

FYSA I keep the unmodified RTT sources maintained and properly tagged here: https://github.com/adfernandes/segger-rtt (been doing it for a while)

I'm reasonably certain that Segger doesn't "maintain" any release of RTT outside of the JLink releases. Other projects that Segger does follow, like the Zephyr RTOS from the Linux Foundation, use relatively old libraries, too.

The attitude, much like the SEGGERMicro repository linked above, is to do a code dump "once in a while", ignoring bugfix and patches.

So about once a month (waves hands in the air) I download the latest Segger JLink software distribution and upload to my repository.

@kilograham kilograham added this to the 1.6.0 milestone Nov 24, 2022
@rgrr
Copy link

rgrr commented Mar 11, 2023

This PR has been added to the 1.6.0 milestone. Anything speaking against merging this into develop in the not too far future?

@adfernandes
Copy link
Contributor

adfernandes commented Apr 16, 2023

FWIW, I just pushed Version V7.86h (2023-04-12) to my unmodified, pristine mirror of Seggger's RTT distribution.

Segger's release notes are usually pretty complete, and there were some minor changes to to RTT backend in recent history.

@anaoum I would ask (please!) that the most recent-possible RTT sources, like mine from above, be merged into the Pico SDK since this can make a difference as the debuggers and protocol (slowly) evolve!

@kilograham
Copy link
Contributor

This PR has been added to the 1.6.0 milestone. Anything speaking against merging this into develop in the not too far future?

I think we would want to avoid copying a bunch of Segger code into this repo... so would prefer to build with code from a submodule and have that submodule be optional

@adfernandes
Copy link
Contributor

@kilograham, I agree with your "use a submodule" sentiment 100%

The issue is that there is no official source, as far as I know, RTT repository - that's why I maintain this, which is very carefully exactly what Segger distributes.

@anaoum, if you agree, I'd be happy to rework your PR to use a submodule...

@anaoum
Copy link
Author

anaoum commented May 31, 2023

@anaoum, if you agree, I'd be happy to rework your PR to use a submodule...

no objection from me. I would have done it myself but am quite time poor at the moment.

adfernandes added a commit to adfernandes/pico-sdk that referenced this pull request Jun 4, 2023
adfernandes added a commit to adfernandes/pico-sdk that referenced this pull request Jun 4, 2023
adfernandes added a commit to adfernandes/pico-sdk that referenced this pull request Jun 4, 2023
adfernandes added a commit to adfernandes/pico-sdk that referenced this pull request Jun 4, 2023
adfernandes added a commit to adfernandes/pico-sdk that referenced this pull request Jun 4, 2023
adfernandes added a commit to adfernandes/pico-sdk that referenced this pull request Jun 4, 2023
adfernandes added a commit to adfernandes/pico-sdk that referenced this pull request Jun 4, 2023
adfernandes added a commit to adfernandes/pico-sdk that referenced this pull request Jun 4, 2023
adfernandes added a commit to adfernandes/pico-sdk that referenced this pull request Jun 4, 2023
adfernandes added a commit to adfernandes/pico-sdk that referenced this pull request Jun 4, 2023
adfernandes added a commit to adfernandes/pico-sdk that referenced this pull request Jun 4, 2023
adfernandes added a commit to adfernandes/pico-sdk that referenced this pull request Jun 4, 2023
@adfernandes
Copy link
Contributor

(ping @kilograham and @anaoum)

Superseded by #1411

@adfernandes
Copy link
Contributor

@kilograham, I have vendored in RTT and removed the submodule.

However, no matter what I do, I cannot apply the compiler flags.

set_source_files_properties(SEGGER/RTT/SEGGER_RTT.c
    PROPERTIES COMPILE_OPTIONS "-Wno-cast-qual;-Wno-cast-align")

You'll see that I have

include(CMakePrintHelpers)

cmake_print_properties(
    SOURCES SEGGER/RTT/SEGGER_RTT.c
    PROPERTIES COMPILE_OPTIONS
)

and cmake does appear to apply those properties, but ... they're not in the actual compile commands, and the build fails.

Any idea what might be wrong?

@peterharperuk
Copy link
Contributor

You'll have to specify the full path to the source file for set_source_files_properties to take effect.

@peterharperuk
Copy link
Contributor

I pushed my suggested changes that seem to fix the issue - hope that was ok


#if LIB_PICO_STDIO_RTT
stdio_rtt_init();
#endif
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing "rc = true;"?

@rgrr
Copy link

rgrr commented Jun 7, 2023

question: why don't you put the SysView files into the SEGGER subdirectory. Those are part of the more or less generic RTT support and would allow a user to easyly integrate SysView support.

@kilograham
Copy link
Contributor

closed in favor of #1411

@kilograham kilograham closed this Jun 7, 2023
@kilograham kilograham removed this from the 1.6.0 milestone May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants