Library Approach for LIBSTLINK

[a-michelis]

This Pull Request is purely for overview purposes.

This PR attempts to give STLink project a proper library approach, to enable users incorporate libstlink into their projects.

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 sup-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 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.
• Exhaustive testing
• Documentation of the exported functions (and of course evaluate whether the overall exposure is excessive or too limited).

A.M.

[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

[Nightwalker-87]

@a-michelis: It might be a good idea to do some refactoring related to STLINK commands and related code on STLINK programmer APIs (see #1399) in the sense of a library approach, before continuing with more general restructuring of the code base. As you may have already noticed, I pushed this Draft-PR to the v1.8.2 milestone in the meanwhile.

[a-michelis]

@a-michelis: It might be a good idea to do some refactoring related to STLINK commands and related code on STLINK programmer APIs (see #1399) in the sense of a library approach, before continuing with more general restructuring of the code base. As you may have already noticed, I pushed this Draft-PR to the v1.8.2 milestone in the meanwhile.

I had the same concept in my mind, initiated from a different prespective:

I started playing around with the logging facility of the project. Specifically I'm trying to create the logging interface I was talking about in #1437 (comment) . The approach I'm working on is:

1. 4 Distinct log types:

   • ERROR
   • WARNING
   • INFO
   • DEBUG

   Debug category is a phony for Tracing. 10 individual trace levels will be used, from 0 to 9.

   enum log_type {
       LT_NONE = 0, // default value, no log
       LT_ERR = 1,
       LT_WRN = 2,
       LT_INF = 3,
       LT_DBG = 4, // this is a trace. tracelevel = value - 4, valid tracelevel values are 0-9. everything above that will be clipped and handled as trace lvl 9
       // anything above that is clipped to 13 and handled as level 9 trace
   };

2. Log and trace level is defined programmatically (and can be exported in flags):

   • One can define the max logging level as before
   • If LT_DEBUG is selected, user can also select the Debug logs verbosity (trace level) from 0 to 9.

3. User of the library can:

   • disable logging altogheter or (re)enable it at any given point. (stlink_nolog() / stlink_yeslog())
   • define a custom log handling function (ie for when making a GUI tool and want to export the logs to the tool, or when they want to stream the logs within a trace facility via sockets or something)
   • change the log handling function at any given point in execution time (stlink_loghandler_custom(void (*handler)(uint8_t lvl, const char* sender, const char* msg)))

   Logging disabling can be also usefull to the tools, for a convinient -q / --quiet flag.
   Even when using custom log handling, the function will receive only the logs qualified by the loglevel and tracelevel filters.

4. There are three in-house (aka default) log handling functions.

   • STDOUT logging (as before), with format:

     [LOGTYPE] TIMESTAMP | SENDER_FUNCTION | MESSAGE
     

   • Color-enabled STDOUT logging: same format as simple stdout, but with different colored trace levels (ADHD fellas like me will find it way easier to read)

   • CSV logging : Exports the logs to a CSV file with logtype, timestamp, sender and message columns

   All of these can be selected by using flags (--color, --csv $FILEPATH) in the tools, or programmatically, by using stlink_loghandler_default() / stlink_loghandler_default_color() stlink_loghandler_default_csv(const char* filepath). Any of the handler changing functions will first check if a csv file is opened by a previously used CSV handler and close it.

   The log type codes will be ERR, WRN, INF and TR0-TR9

Reasoning

I strongly believe that having a robust logging system will allow us to better understand an issue, while letting us write a more structured code - and thus maintain it easier (want to understand what a function does? Simply read its log/trace commands). Also, having a documented train of thought via the log/trace entries, will facilitate API documentation writeups later on, when the Library approach comes to life.

All that said, If you find a part in the codebase that needs immediate care, please share proposed approaches/changes so i can start with them instead!

Off topic

On another note, today is my bday 🎉 , so my presence will be somewhat limited -I'll start working on it the lib more intensively the following week's aftenoons.

[Nightwalker-87]

Happy birthday 🎉 😃. Have a nice day.

Thanks for sharing your ideas which were interesting to read.
Here are my thoughts, attempts and findings:

a) To ensure proper logging is very useful and not only convenient. Surely we can put that up on the agenda. Currently there are two kinds of logging available. The standard one is based on uglylogging and the second one (optional) is spdlog for which a wrapper file (logging_spdlog_wr.h) is present in the sources. The latter can be enabled in logging.h.

b) As you can see, I've reallocated and renamed some header files in the last commit to improve the current structure. Also the source files common.c and sg.c have been renamed, along with their respective header files, to highlight the oldest and not yet restructured, reimplemented or optimised parts of the codebase.

c) In the meanwhile I found that st-info currently outputs
st-info: error while loading shared libraries: libstlink.so.1: cannot open shared object file: No such file or directory though the build test all pass successfully. Returning to older commits, for which this still worked in the past, give the same result. Can you try to reproduce this on your side as soon as you find time for that? I just want to make sure that we do not accidentally introduce a regression bug. Maybe something just messed up on my side.

Looking at this, I'd suggest to line that up in a c) --> b) --> a) approach, which should still go to the v1.8.1 milestone, especially c) and b).

[a-michelis]

about c

Windows, after install:

Linux comming soon

[Nightwalker-87]

I found the reason for this. As it appears, there is no regression.
sudo ldconfig did the job.
I'll add a HowTo-note about it to our tutorial soon.
Edit: In fact this was already covered by the compiling instructions. 👀 🙈

[Nightwalker-87]

Having sorted quite a few things out in the codebase, I think we should now be ready for logging improvements.
I tried to implement the recent changes carefully and step by step followed by a few very basic tests afterwards.
Some quick interim testing of main tasks and functions of somewhat daily use on another system could turn out helpful though.

[a-michelis]

I'm sorry for being MIA,
Im struggling to find some Free time lately... That said I started implementing a new logging system from scratch, but it's still in progress, that's why i Haven't pushed anything to my fork yet

[a-michelis]

Hello again, after all this time!

I managed to test my approach using a "hijack-ey" method, just to try things out. Seems Simple and Helpful.

Next step is modifying the rest of the library to use this 😝