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 logging feature #10

Merged
merged 22 commits into from
Feb 7, 2023
Merged

Add logging feature #10

merged 22 commits into from
Feb 7, 2023

Conversation

RiccardoGrieco
Copy link
Member

As per title, this PR adds the text logging utility to the BAF library. In addition, it also adds CI build workflows.

The feature relies on the [bipedal-locomotion-framework](https://github.com/ami-iit/bipedal-locomotion-framework)'s one, just changing the name/prefix of the logged message to baf instead of blf.

More specifically, the feature relies on the following PR of blf which is merged but not yet released: ami-iit/bipedal-locomotion-framework#582.
For this reason the tag used is the commit c1cb3fa3329cae7515de6d453bc099dde2cd0645 and the CI, which mostly relies on conda-based dependencies, installs blf from source.
Related TODOs will be removed once after the next version of blf is released.

@RiccardoGrieco RiccardoGrieco self-assigned this Jan 24, 2023
This was referenced Jan 24, 2023
@RiccardoGrieco RiccardoGrieco force-pushed the feature/add-logging branch 15 times, most recently from 1a3f284 to f42510d Compare January 25, 2023 15:15
@RiccardoGrieco
Copy link
Member Author

Somehow I managed to find the cause of the failure on windows https://github.com/ami-iit/biomechanical-analysis-framework/actions/runs/4042357636/jobs/6949962545#step:11:78.

Turns out that Windows paths have a restriction of 260 chars (see here).

Now, the file that is being attempted to generate in the step above mentioned is: D:/a/biomechanical-analysis-framework/biomechanical-analysis-framework/bipedal-locomotion-framework/build/src/YarpUtilities/CMakeFiles/yarp_idl_to_dir/thrifts/BipedalLocomotion/YarpUtilities/vectorscollection//BipedalLocomotion/YarpUtilities/VectorsCollection.cpp, which has 263 chars.

In 6c943a7 I shortened the name of the directory where bipedal-locomotion-framework is cloned to blf to fix this issue.

@RiccardoGrieco RiccardoGrieco force-pushed the feature/add-logging branch 2 times, most recently from d5cc4a9 to a9531b6 Compare January 30, 2023 17:47
@RiccardoGrieco
Copy link
Member Author

RiccardoGrieco commented Feb 2, 2023

Finally all checks are passing! @lrapetti can you proceed with the review?

Copy link
Member

@lrapetti lrapetti left a comment

Choose a reason for hiding this comment

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

The logger and initial skeleton are there, for me we can merge.

I don't know if it makes any sense to add a line in the CHANGELOG such that at the first release of the library, we have a list of features.

@RiccardoGrieco
Copy link
Member Author

The logger and initial skeleton are there, for me we can merge.

I don't know if it makes any sense to add a line in the CHANGELOG such that at the first release of the library, we have a list of features.

Done @lrapetti. Thanks for the review, merging.

@RiccardoGrieco RiccardoGrieco merged commit 9ba1a04 into main Feb 7, 2023
@RiccardoGrieco RiccardoGrieco deleted the feature/add-logging branch February 7, 2023 11:07
@DanielePucci
Copy link
Member

Super cool, first PR with implementation elements, well done!

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.

3 participants