BLE NUS - text console over BLE

[hubmartin]

I used this BLE NUS uart console on other embedded NRF52 projects and it is really handy. You can log errors over BLE and debug your app.
To display data from NRF chip you need an app, or web browser.

Right now the firmware advertises has BLE NUS characteristic, can receive packet and replies the same data back. So low-level functionality is there, now I need to help with integration. More info under screenshots.

Console might be a way for not so skilled users to do some minor watch configuration which might not be possible on small screen. For example size, color and positions of watchface elements. Editing config, list and edit files in LittleFS. All that without reflashing, which is somehow hard for beginners to create a new watchface.

(sorry for big pictures, please see text in between them)

You can use NRF Toolbox app on phone (select UART, swipe right to see console, at the end of the command add newline before pressing SEND, see https://devzone.nordicsemi.com/f/nordic-q-a/33687/nrf-toolbox-2-6-0-uart-does-not-send-lf-cr-or-cr-lf-as-eol)

Bluefruit app from Adafruit

or use some WEBBLE (Web BLE) terminal console https://github.com/makerdiary/web-device-cli (not supported anymore, this sends single characters with remote echo, too much complex).

WEB BLE terminal working in any chromium-based browser
https://terminal.hardwario.com/

Whan now needs to be done is some object oriented C++ which is not my cup of tea. I'm struggling how to divide this because I see that in the code there is dependency injection in constructors. This is too high level for me so I'll be glad to hear how to partition BleNus class and create some Console class. Some code-prototypes would be better then explain me in C++ lingo what to do, I'm still learning C++.

Console class could be created in SystemTask and be somehow connected to BleNus which will send received characters to Console class over some function. These cahracters will be buffered (what OS primitives use to this?) until new line is received and command is parsed, executed and response send back to client).

Console will also pass filesystem fs and lvgl dependency so later console could list files, change color or position of LVGL objects on screen.

Bonus points

Also Console might have some static (hope I use this term right) class which will send also all debug info from all parts of the firmware where logs are used. Right now all goes to the NRF_LOG_INFO macro, but we might create new function log_info which will send data to RTT NRF_LOG_INFO and also to some Console.printf() function. So you can see logs also over BLE. And I'm not sure how all this glue together. So any tips are welcome. I do not have plenty of time so the easisest solution would win. I would like to close this PR in the state of working console with some basic commandds. The NRF_LOG_INFO redirection might be part of another PR in future.

Thanks

[hubmartin]

What C++ feature I should use to buffer received characters from BLE? Nimble sends bytes in mbuf. In C I use char arrays, but I'm sure C++ has some stdlib feature in its sleeve. If you can point me to some example code or similar feature already somewhere in the firmware it would be great.

[hubmartin]

Another question. The data arrives in Nimble thread context? I see that different classes uses its Process() function in SystemTask loop. I guess it is to save RAM resources from too many tasks and stacks. So I will process the console also in SystemTask.

[hubmartin]

Can anyone from the experienced C++ developers help me please? @JF002 @Avamander @Riksu9000 @kieranc
I'm stuck for many hours with some stupid class definition mistake and cannot compile code. I'm getting really frustrated, and I'm sure for someone skilled in C++ it could take few minutes to find the issue. I'm a C developer and I'm learning C++ with this project and there's nobody else around me to help. :(
I've added BLE console which works, now I'd like to add the Console class, but even I followed the same way the NimbleController class is defined and added new source files to CmakeLists.txt, it looks like I'm missing something

In file included from /home/martin/Desktop/dev/watches/InfiniTime/src/components/console/Console.h:7,
                 from /home/martin/Desktop/dev/watches/InfiniTime/src/components/console/Console.cpp:2:
/home/martin/Desktop/dev/watches/InfiniTime/src/systemtask/SystemTask.h:118:29: error: 'Console' in namespace 'Pinetime::Components' does not name a type
  118 |       Pinetime::Components::Console console;
      |                             ^~~~~~~

The latest push has this issue.
Thanks for your help.

[Riksu9000]

It's because of this line in Console.h
#include "systemtask/SystemTask.h"

Console requires systemtask but systemtask requires Console.

[hubmartin]

@Riksu9000 Thanks a lot (I wish I asked few deays earlier). It makes perfect sense now. Now I can move a bit. Thanks.

[hubmartin]

Ugly concept is working, added console.Print() command to SystemTask and getting results in Web BLE terminal.

TODO:

• transmit buffering, BLE NUS buffer might not send longer strings
• receiving buffering to receive incoming command and parse it
• Figure out how to integrate this console thing so you can call it from anywhere.
• Find out how and add option for routing NRF_LOG_INFO also to console.Print
• Find or create better terminal than https://wiki.makerdiary.com/web-device-cli/. We do not need sending every character separately and also remote echo is not efficient. Single input line with sending on enter and textarea with response will be best.

[Avamander]

You can turn it into a logging backend now. There is a nice example there already.

[hubmartin]

You can turn it into a logging backend now. There is a nice example there already.

Can you be more specific please? Do you mean NRF logging and adding it by nrf_log_backend_add ? I keep that for later or someone else. Seems big bite for me now.

[Avamander]

Check the logging folder and the relevant classes. It's really not a big bite :D.

[hubmartin]

@Avamander I did, but then I run into error. What I understand is that NRF lib have different order of items in structure than in initializer? Is this issue of the NRF or InfiniTime project which has set errors for some warnings like this?
Seems like bad idea to fix it in NRF lib, is there some workaround?

I shoved parts of nrf backend registration to System task to see if I at least could it compile. Not sure I would be sucesfull.

[hubmartin]

Hmm, I'm stuck again. I can send data to bleNus to send text to computer, but I have no idea how to properly connect it in the oposite way, so received characters over BLE goes to console class.

In the image below I'm passing to console class also nimbleController which contains BleNus class responsible for communication. So when I call console.Print it goes to nimbleController.Print() which calls bleNus.Print (this is also ugly, ready for suggestions).

And in the oposite way I have no idea how to properly with namespaces and classes connect so BleNus class which receives a character over BLE will pass it down through parent nimbleController to the systemTask whic contains console.Received() which adds bytes to RX buffer.

In C I would pass pointer to the function, but I'm sure there are different patterns in C++ how to deal with this.

SystemTask.cpp constructor

I drafted how terrible the situation is now with Print going over nimbleController which is also ugly. But how to connect it the other way from BleNus to the Console?

Thanks for help @JF002 @Avamander @Riksu9000.

[hubmartin]

It somehow works :)
https://youtu.be/pN13IN-AXJ4

I need to clean the code now.
I used std:function to register Console function callback for the BLE NUS.

Now there are few example commands, I'll keep some of them but only with basic functionality. I hope that others will build on this PR and create better interface and functionality.

[hubmartin]

I've added ACC command to print X,Y,Z accelerometer data. Justa concept so we could for example create more robust wake on wrist

[hubmartin]

@happyme531 I Agree, but this is out of my scope, time and C++ abilities. I guess the final goal should be to add this as a another NRF logger backend but this has to be done by someone else who will build on my PR.

[hubmartin]

Hi @JF002, it should be minimalistic now

• Lowered 256 byte buffer to 32 byte
• Removed calls to console.Print() from SystemTask so it doesn't send anything in release code
• I would keep the console in the main build. The RAM footprint is now around 200 bytes, the flash could be lower than previous 1500 bytes since I removed calls to console.Print.I don't feel like I could manage & keep alive this PR any longer if another requests arise. Not much time in these days.

Now the code doesn't eat practically any resources, but it is ready for developers to use it in their builds to find bugs.
Later the NRF log could be enhanced and this console could be used as another backend, but this is out of scope of this PR.

(I still see that @Avamander requested a change in this PR (months ago), but when I open the requests, they are already outdated. Is this something I have to take care of or I could ignore that? Thanks)

[Avamander]

All the files need a reformat using clang-tidy.

[Avamander]

Now the review should be more correct.

[hubmartin]

@Avamander I was afraid someone will ask for clang-format. I asked before in the chat how to parse it but I forgot. Can you guide me please to the correct parameters? It was somewhere in git pre-commit triggers, not sure where I seen that.
It was clang-format, is there some new clang-tidy command?

If this step with clang is necessary, I would like to ask anyone who knows more than I to add instructions to the docs. Thanks.

[Avamander]

Can you guide me please to the correct parameters?

Check the git commit pre-hook in the hooks folder.

[hubmartin]

@Avamander Is it allright now? I clang-formatted just the files I've newly created in my PR.

[Avamander]

@hubmartin

Is it allright now? I clang-formatted just the files I've newly created in my PR.

Much better, thanks. Did you also check out clang-tidy and compiler warnings (you might have to enable them both separately), any you could fix?

Fixed

[hubmartin]

@Avamander I'm not familiar with clang-tidy. What do I need to do exactly?
In normal compilation I did not see any warnings. Also in the CI build there aren't any. Is it because they are somehow restrained?
https://github.com/InfiniTimeOrg/InfiniTime/runs/3980287640?check_suite_focus=true#step:17:1

[Avamander]

I'm not familiar with clang-tidy. What do I need to do exactly?

clang-tidy [src/path/file.cpp] in the root folder of the project where the configuration file is located, then it uses it.

Most editors also have integrations that can display the warnings nicely inline.

In normal compilation I did not see any warnings.

I think they're only displayed once per clean build (for rebuilt files).

[hubmartin]

@Avamander Then there are no warnings. Since CI is clean build. And I'm sure I fixed them in the begining, I don't like warnings, do you see any?
I'll be few days away so clang-tidy later.. Can you explain what it does? Why is not clang-format enough? Thanks.

[Avamander]

@hubmartin

And I'm sure I fixed them in the begining, I don't like warnings, do you see any?

Great!

I'll be few days away so clang-tidy later.. Can you explain what it does? Why is not clang-format enough? Thanks.

clang-format is an autoformatter, clang-tidy is a linter - it checks for common mistakes, bad patterns. Helps increase the code quality and keep the codebase more approachable to new contributors.

[hubmartin]

@Avamander fixed some stuff according to clang-tidy suggestions.

[hubmartin]

Hi @JF002, I did all the suggestions from @Avamander. Let me know if it could be merged in current state. Thanks.

[hubmartin]

Hi, another month passed with no response from anyone. Since I'm not so active now in InfiniTime development I guess that I woulnd't be able to babysit this PR to be without conflicts all the time, since it connects to SystemTask.cpp which is changed quite often.
Let me please know if this PR will be merged or not. I did all the clang formatting and cleanup.
Currently this BLE console does not emit any messages, has small footprint to be in main build, but it is ready for developers to build on it and debug stuff.
Looking for answer from anyone @JF002 @Avamander @geekbozu
Thanks.

[Riksu9000]

I'm not sure we really have a need for this. Since this wouldn't be used by end users, It's basically just for devs who don't want to open their watch or buy a devkit, but also want logs. I don't think this is many people. InfiniSim has logs. I've developed on a sealed watch, but it's slow and we don't recommend developing on a sealed watch anyway, so why would we provide a feature specifically to make that easier?

[JF002]

@hubmartin I'm really sorry for not providing any feedback for so much time. I don't have any excuse except that I'm completely overwhelmed by the number of PRs to review :/

I'm 100% convinced that this feature could be really useful fore many "alternative/niche" use-cases : debug logs, data logging, integrated command line interpreter, integration with 3rd party apps,... It could also be uses to extract more data from the motion and HR sensor to help design better algorithm to process the data, and probably many more things.

However, most of those use-cases are targeted towards developers, hacker and tinkerers and do not fit well with our (new) project vision. Since we want to focus on this vision, I guess we'll have to do without this feature, unfortunately. But this could be the basis of an InfiniTime variant that target specifically those use-cases, of course!