[feature] New logging system in preparation for an intended stlink library approach

[a-michelis]

This Pull Request is purely for overview purposes.

This PR 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 now uses libusb-cmake project as its libusb provider, which in turn allows for custom-built libusb, tailored to each system's needs.
• All included headers have been placed into appropriate stlink sub-folders that are included into the project as system includes, to allow for system-like includes. This change required 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 (this PR):

1. Changing the logging infrastructure to allow programmatically managed logging, instead of STDOUT logs.
2. 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
3. Documentation of the exported functions (and of course evaluate whether the overall exposure is excessive or too limited).

A.M.
(updated by Nightwalker-87)

[Nightwalker-87]

There are a few header dependencies that have not been converted to the new scheme. Thus, as it appears, this results in compilation errors. However I believe this can be easily fixed. I'll continue reviewing afterwards. Yet there are quite a few attempts and improvements I consider very useful.

[Nightwalker-87]

I'd prefer to keep source and header files together in the same directory respectively, unless there are superior header files without a matching source file, but this should be rather a stylistic issue than a technical one...

[a-michelis]

I'd prefer to keep source and header files together in the same directory respectively, unless there are superior header files without a matching source file, but this should be rather a stylistic issue than a technical one...

I tend to divide the big projects into separate, smaller, target-centric sub-projects, included into the root CMakeLists.txt via add_subdirectory(${PROJECT_SOURCE_DIR}/src/target_name). This usually helps with projects like stlink that generate a toolset rather than one entity -allows for separation of concerns and enables two developers work in two different tools at the same time while avoiding conflicts.

Additionally, I usually separate headers and source files (despite being harder to manage) because it makes it easier to prefix different versions differently if ever needed, or avoid name conflicts, like with Windows and existing libusb.h.
AFAIK, there is no mechanism in CMAKE that allows for "virtual prefixing" of headers, which in turn makes using <stlink/header.h> rather than "header.h" in our sources and headers impossible.

That said, the project's structure is already in place and I'm in no position to enforce such changes, unless they're wanted -in which case I can pay closer attention on what goes where, how and why. :)

---

Seems like separating sources and headers makes it harder for static analyzers such as cppcheck to properly analyze the project, which is also something to consider in my approach.

[a-michelis]

Fixing the non-prefixed usb.h made both workflows succeed - i shall remove my branch from the workflows, tho, since it will mess up the main project.

That said, I also added windows (MSVC) on the workflow and that passed as well!

[Nightwalker-87]

Fixing the non-prefixed usb.h made both workflows succeed - i shall remove my branch from the workflows, tho, since it will mess up the main project.

That said, I also added windows (MSVC) on the workflow and that passed as well!

No, it won't mess up the project in any kind, as long as the merge of a test-failing PR does not take place. Indeed it is even better to have them here to detect possible issues before merging into the testing branch.

[Nightwalker-87]

Further I've converted this PR into a draft as long as proposed changes are still in discussion.
Remember: Haste makes waste - we are not in a rush. Approaches and ideas should be well considered by taking into account relevant aspects. This does not imply that there is no view to improvements at all.

[Nightwalker-87]

Any update here?

[a-michelis]

Hello again,

Unfortunatelly IRL is being crazy for the past month.

Let's formulate a plan as to what needs to be implemented and at which order, so I can allocate time implementing, instead of theorizing about potential features!

To my experience, making libstlink a standalone library requires:

1. a bulletproof method of linking to either existing or downloaded/compiled LIBSTLINK. Currently, this is not present for Windows/MSVC.

2. complete separation of the library source/headers and the rest of the toolset (as in subproject, not necessarily a dedicated repo)

3. a bulletproof method of providing bindings to the library (ie FindStlink.cmake, pkgconfig etc)

About these points:

(1) even though we have a provider for Windows/MSVC LIBSTLINK, it seems to require some manual labor which we shall take into account:

• Having the LIBUSB downloaded/built for Win/MSVC requires from STLINK to "install" it together with its own binaris -which counts as clear redistribution and I'm not aware of its legal implications.
• Requiring it to be pre-installed might be a solution, but it limits the libraries portability -which i'm not aware whether it's a wanted quality.

(2) To add to this single-responsibility fashion, I'd suggest each tool to be contained within its own subproject for better maintainability and proper linking.

(3) This is also related to note (1), and requires some research and further testing on how this library is supposed to be used. If portability is on the table, the methodology must also contain the discovery logic (such as StlinkConfig.cmake ). If not, a simple FindStlink.cmake with some options available about the requested library type could be suffice.

All these said, I can only lay down some ideas and contribute to their implementation to the best of my ability! I wish you find this textwall usefull and it allows you to cherrypick the next actions!

Cheers,
A.M.

[Nightwalker-87]

Hello again,

Unfortunatelly IRL is being crazy for the past month.

Let's formulate a plan as to what needs to be implemented and at which order, so I can allocate time implementing, instead of theorizing about potential features!

To my experience, making libstlink a standalone library requires:

1. a bulletproof method of linking to either existing or downloaded/compiled LIBSTLINK. Currently, this is not present for Windows/MSVC.

2. complete separation of the library source/headers and the rest of the toolset (as in subproject, not necessarily a dedicated repo)

3. a bulletproof method of providing bindings to the library (ie `FindStlink.cmake`, pkgconfig etc)

About these points:

(1) even though we have a provider for Windows/MSVC LIBSTLINK, it seems to require some manual labor which we shall take into account:

* Having the LIBUSB downloaded/built for Win/MSVC requires from STLINK to "install" it together with its own binaris -which counts as clear redistribution and I'm not aware of its legal implications.

* Requiring it to be pre-installed might be a solution, but it limits the libraries portability -which i'm not aware whether it's a wanted quality.

I can't spot a problem here, as libusb is an open project as well - I mean it's a library to be used with other software, so what's the point about it? The only thing one should take into account FMPOV is that redistribution occurs under the same licensing terms of the original project for the redistributed part.

(2) To add to this single-responsibility fashion, I'd suggest each tool to be contained within its own subproject for better maintainability and proper linking.

A separate subdirectory structure should do. As this solely applies to Windows anyway, things should go to /stlink/src/win32/ which appears to be the appropriate place.

(3) This is also related to note (1), and requires some research and further testing on how this library is supposed to be used. If portability is on the table, the methodology must also contain the discovery logic (such as StlinkConfig.cmake ). If not, a simple FindStlink.cmake with some options available about the requested library type could be suffice.

The latter seems to be the most sufficient approach.

All these said, I can only lay down some ideas and contribute to their implementation to the best of my ability! I wish you find this textwall usefull and it allows you to cherrypick the next actions!

I consider it a good approach to start with the PR you recently opened, tailoring it down to Windows-specific changes in the context of the given comments from the review today.
In a second step I'd like to address the cross-building capability under the changed circumstances.

After that it may turn out to useful to strip this Draft-PR down to the remaining changes in order to continue the review.
I'd not change the structure of the project yet, as I see this as a separate, independent issue, which may follow in a third step.

Kind regards
Nightwalker-87

[a-michelis]

Hello again, it's been a while...

Since my wedding is approaches i have less and less time to give to myself (and, thus, the project). I promise to start actively working on it after after the commotion ends (starting early July) and keeping you posted with frequent updates.

In general I'm interested in maintaining the project more holistically and not just provide "satelite features". That said, I do believe that some structural changes must take place first to allow for modularity and possibly better maintainability.

The hardest issue so far is understanding the inner code workings, so, proper (probably in-code) documentation of the implemented functions would be helpfull...

When the time comes (aka. when the wedding is done), If you could give me a tour to the codebase it would be godsent!

Cheers,
AM.

[a-michelis]

I'm officially married! Starting from the following weekend, I'm planning to start diving into the codebase and write some notes about how it works!
If there's any resource on the matter, or even suggestions on how to tackle it, I'd gladly take it.

Quick questions:

• I'm thinking in-code documentation with some 3rd party tool to extract it and create proper documentation out of it. Any ideas, restrictions or prefferences on that? I've used in-code documentation for my C# Projects before (DocFX) but not for C/C++ libraries.
• How welcome is C++ code on the codebase? If applicable, what are the restrictions/requirements for C++ code?
• Do you have a preference on what should be implemented when (in terms of wanted tasks and in terms of order)?
• Do you prefer this thread for further discussions/chit-chat or some other way is more appropriate?

Cheers!
A.M.

[Nightwalker-87]

@a-michelis: Hi. Good to hear from you.
Congratulations, all the best wishes and may there be many happy returns of this special day.

Regarding the topic above there has been no further action yet as I could not find any spare time in the recent weeks and months to invest in this project. 😞

Regarding you questions:

• Maybe it's a good idea for now to solely focus on the logging feature you originally proposed.
• We can keep this place for further conversation, but I'd suggest to update this PR to the latest commit of the testing branch, so it can continue to be actively used.
• C++ Code is indeed welcome, especially in places where it helps to improve structuring (understandability), code safety and maintenance of the code base. Assembly for very hardware-near implementations has already has found it's way into the code in some parts.
• We at least used to have an option to use pandoc. I'm not sure though, if this option it is still completely functional. doxygen may also be worth a discussion later on.
• In-code-documentation: In the end I'd love to have a common source file header formatting as I got to know from other projects characterising the respective source and/or header file, which can be then automatically detected by such tools as mentioned above. Functions should be introduced by a short description together with tags like "@ param", "@ return".

I hope this draws a first idea in which direction one may proceed step by step.

Regards
nightwalker-87

[a-michelis]

I've started my logging approach from scratch - since I've done some reading on spdlog - to properly solidify the situation.

Some key factors are:

• The approach now have three main "logging init" functions, representing 3 different spdlog sinks:

  • logger_init_stderr : logs into STDERR with colors (custom STDERR sink with color support)
  • logger_init_file : logs into a specified file (default spdlog file sink)
  • logger_init_callback : this won't be used by the existing tools, but can later be used by external software which uses stlink library to get structured logging entries. it's a custom sink that, upon receiving a new log, it triggers the callback specified during initialization (which uses simple enough argument types to support interoperability with other languages such as C#)

• All logs contain:

  • Timestamp
  • LOC (File, Function, Line)
  • Log Level
  • Message

• the step i'm currently working on is removing ugly logging completely and, since I realized that the log levels seem to correspond to stlink's log levels, to bridge the value gap the same way you did with libusb-stlink log compatibility.

• Next step is to provide each device with its own logger (and, thus, its own max logging level), so we can instatiate multiple devices at once without issues and without a common logging level. This needs carefull planning, since it requires structural changes into the stlink_t struct.

I suppose that, since I re-iterated the logging system, there now is a new branch which requires a new PR. If i'm wrong, please advice on how to proceed!

If you want to see the current state of logging, I'm about to push the first changes into my forked repo.

Cheers,
A.M.

[a-michelis]

Side note: I'd gladly convert the whole codebase in C++17 or C++11, but that'll take time.
That's something I plan to do for fun with my fork at some point either way, so, if it's something welcome to the original project, I guess it wouldn't hurt to make a PR or even start the project as a separate repo in the org.

[Nightwalker-87]

C++ 17 and C 17 are preferred. GCC 8.1 or later should be for compilation in terms of upcoming system requirements.
A Feature-/Maintenance-PR should be fine.

[Nightwalker-87]

The above discussion is going to be continued in issue #1474.
This PR is closed instead as it is outdated in relation to the current state of code development.