[feature] New logging system in preparation for an intended stlink library approach
This issue attempts to give STLink project a proper logging system in preparation for a library approach, to enable users incorporate stlink-lib into their projects. Proposal: Main changes (in a nutshell)
- The project may use libusb-cmake project as its libusb provider, which in turn allows for custom-built libusb, tailored to each system's needs.
- All included headers may be placed into appropriate stlink sub-folders that are included into the project as system includes, to allow for system-like includes. This change requires the change of all inclusions in the project to change from #include "%HEADER%.h" to #include <stlink/%HEADER%.h>
- Several headers have exposed functionality which could be used by custom software, using extern "C" blocks.
Much needed prior additions that are missing:
- Changing the logging infrastructure to allow programmatically managed logging, instead of STDOUT logs.
- Help to accommodate the appropriate cmake modifications for other BSD/*NIX based systems, as well as verify/implement a proper handling of pre-installed libusb followed by exhaustive testing
- Documentation of the exported functions (and of course evaluate whether the overall exposure is excessive or too limited).
This issue succeeds PR #1437 in order to continue the open discussion.
A.M.
Update:
I managed to borrow an STM32H753ZI from my work place and will start testing today.
Cheers!
Original Reply
As it stands I'm nearly done preparing the PR, but as my last STM32 Dev board got fried (its power distribution chip, to be exact), I lack the means to test it 😝
I'll push the latest changes later today into my fork and probably start a new PR to give others the chance to test it as well... The only thing I'm lacking right now is a way to incorporate the "file" output as a flag, within the tools (haven't dug deep enough to know how args are parsed)
We're getting somewhere:
Coloring now works
As seen in the pictures above, I managed to implement the Logging system itself (as a separate shared library) , but I have some questions about the inner workings of ST-Link device instances:
-
Each ST-Link instance seem to contain its own verbosity level. That said,
openimplementations, seem to call a global (?) initializer. Does the current logging facility only support global logging and this field was added as a placeholder, Or there are distinctions from instance to instance that I missed? -
If no per-instance logging level is supported, would that be something we're interested in?
Sidenote:
As for the C++ re-implementation, the more I dive into the code, the more C++ makes sense to me - It'd make the codebase so much more understandable and maintainable... I'm probably going to give it a try early Aug!
I'll get back to your questions soon. I am currently struggling to find the spare time needed.
No worries at all! In the mean time, I did a PR in libusb-cmake to allow for better integration for stlink in the future (assuming we're going down the CMake overhaul and/or Cpp rewrite route)
1. Each ST-Link instance seem to contain its own verbosity level. That said, `open` implementations, seem to call a _global_ (?) initializer. Does the current logging facility only support global logging and this field was added as a placeholder, Or there are distinctions from instance to instance that I missed?
I assume only global logging is the case, but I'm not definitely sure.
2. If no per-instance logging level is supported, would that be something we're interested in?
Possibly not, unless there is a specific use case you can imagine that may be worth a discussion.
As for the C++ re-implementation, the more I dive into the code, the more C++ makes sense to me - It'd make the codebase so much more understandable and maintainable... I'm probably going to give it a try early Aug!
Much appreciated. Is there any specific basic functionality that would make sense to start with in a step-by-step approach?
I can grant some extended permissions in the project to ease contribution for you.
I'm currently trying to get a SSD1306 display to work in a well defined and understandable way on a STM32 MCU in a basic approach (without using any external libraries), which is a bit cumbersome as one may imagine... :sweat:
I assume only global logging is the case, but I'm not definitely sure.
Then it's probably better to initialize the logging elsewhere (ie. within chip-id retrieval which is a required initialization step and will not be missed by a library usage). This only makes sense in a library, tho, where users might want to instantiate multiple instances, or instanciate the same device more than once during the execution's lifetime. Within the current tooling, this approach does not make any difference - one instance per execution lifetime is guaranteed.
Possibly not, unless there is a specific use case you can imagine that may be worth a discussion.
In a library approach, you may want to distinguish between instances. I.e. I make an IDE plugin with batch-flashing support: I simultaneously have my instance debugged, while (coincidentaly) having multiple connected devices being flashed at the same time.
Now that i think of it more clearly, library verbosity should not make any difference to the end user, It should only be used for dev purposes. So, I'm not entirely sure if per-instance logging would be just overenigeering.
Much appreciated. Is there any specific basic functionality that would make sense to start with in a step-by-step approach?
Sure! Structural boilerplating would definitely be the first step - to create the abstract classes and interfaces necessary for ST-Link functionality.
The way i see it, we currently have the _backend (which essentially acts as an interface template to be populated with function pointers from each implementation), and the _data (don't quote me on the naming) opaque field, which acts as each derived class'es (a.k.a. implementation) private set of members. This is plently compatible with C++ - it simply must be translated to Cpp
stlink_t should be an interface. Its fields should be private members with getters (and setters, if needed). backend should stop exist, and instead pure virtual functions of each func pointer in backend should be implemented within the newly created interface
each implementation will be a derived class. Instead of global functions which are passed by pointer, the implementations will take place using the default override method of C++. This will not only provide a better readability, but also allow for better intelisense when further developing the library.
An additional strong-second step that can be performed is restructuring the existing global, API functions to use the C++ implementations, so we can still provide a reliable C API for interoperabilty.
My libusb-cmake PR seem to indeed be something wanted, but fellows there nudged me towards the right direction to overcome this issue, So I slowly and steadily started the implementation.
Wishing best of luck with your display endeavors!
Cheers, A.M.
I'm looking forward to assist regarding ideas, concepts and maybe even some minor testing approaches by this week. ... as soon as I have my ssh-binding to github back up running. Don't know what killed it really. 😒
I hope you solve the issue soon!
I just pushed a first rough boilerplate cpp abstract class for ST-Link (on my fork, @ cpp_rewrite branch). I need to revisit the exposed (virtual) functions (having guidance as to what shall be exposed would be helpfull there), and I might tone it down a bit with the modernization for easier transition, but I can see this working!
Apart from being busy at work, I have finally managed to restore ssh access to github from my workspace. I'll have a look at your branch and will try to find back into to current state of progress. Looking at the amount of your contribution so far - this will surely take me some time. I will list my thoughts and comments here as soon as I've made it through the code lines.
Suggested changes:
- c2685819cccc317f0eb4f2ddb6b2133cd4b3410b SPDLOG Sinks and logging facility implementation (unfinished)
- eca4f8ff7f90a77f73bf2550ec0c4ee84efdcfbb Polished the ST-Logging subsystem
- Align to License Terms of the project
- Formatting:
- use 4 spaces instead of tabs
- indentation: 4 spaces
- also minimal distance from code-line ending to the beginning of inline comments
- also minimal distance between @ tags with parametres and description in header comments for functions
- open decision: text width in characters per line
- header comments: nice formatting in general; however I suppose to align the "@return" parametres as well
- unify { } formatting in functions: I prefer "1TBS / K&R" over "Classic K&R".
- 2 empty lines between block of includes and beginning of code
- 2 empty lines between file header and #ifndef (in case of header files) or block of includes (in case of source files)
- open decision: formatting of unified file header
- use UTF-8 character encoding and \LF break type (Unix)
- Code:
- good readability and commenting 👍
- appears to be well structured 👍
- open decision: https://www.perforce.com/blog/kw/NASA-rules-for-developing-safety-critical-code <-- Does this appear to be a good attempt or potential overkill?
- Build environment:
- open decision: cmake directory structure and improved compilation / building of the source code on windows and linux including necessary additional packages (I remember an earlier attempt using visual studio on windows, but must admit that I never tested it properly on my VM.)
- a3c5fb7f6f7fe5cf0a58f4d63191788286a593b4 [Intermediate Commit] First steps towards a C++ ST-Link Library --> against the background that we pushed back this attempt in favour of implementing a proper logging solution, the following comments are to be seen in a more general context:
- the source file
common.c(renamed tocommon_legacy.cin the meanwhile) and the related header filestlink.hshould be dissolved along with general refactoring work. - I really like the idea to introduce a source/header file pair covering all hardware-related functionalities of compatible STLink devices. As to my knowledge this content is currently spread across several code sections and maybe even across different source files. Also there is no clear distinction between several hardware variants. These are:
- STLINK/V1
- STLINK/V2
- STLINK/V2-A
- STLINK/V2-B
- STLINK/V2EC
- STLINK/V2-1
- STLINK-V3SET
- STLINK-V3MODS
- STLINK-V3MINI
- STLINK-V3MINIE
- STLINK-V3E
- STLINK-V3EC
- STLINK-V3PWR
Thus related hardware-related implementations for each variant should be directly discoverable in the source code.
I think this is basically what comes to my mind as a feedback to your recent contributions and hope it turns out to be somehow helpful for any further attempts.
Strongly appreciating your work. Thanks! 🥇 Nightwalker-87