Skip to content

Updated memory management, added unit tests, fixed SonarCloud issues#862

Merged
uweseimet merged 7 commits intodevelopfrom
feature_memory_management
Oct 1, 2022
Merged

Updated memory management, added unit tests, fixed SonarCloud issues#862
uweseimet merged 7 commits intodevelopfrom
feature_memory_management

Conversation

@uweseimet
Copy link
Copy Markdown
Contributor

@uweseimet uweseimet commented Sep 25, 2022

This PR resolves most of the remaining memory management and structural issues. The vast majority of SonarCloud issues is now caused by the horrible code in filesystem.cpp/h.
Testing was done with my standard hardware setup on buster and bullseye, with gcc and clang, and of course with the extended set of unit tests.

@akuker I would also like to merge this PR myself after approval, in order to provide details in the merge message.

@uweseimet uweseimet linked an issue Sep 25, 2022 that may be closed by this pull request
@uweseimet uweseimet marked this pull request as ready for review September 25, 2022 22:22
@akuker
Copy link
Copy Markdown
Member

akuker commented Oct 1, 2022

Thanks @uweseimet !! Just a couple of comments.

I know I'm slow, but could you avoid changes to gpiobus for the time being? (unless they're bug fixes)

@uweseimet
Copy link
Copy Markdown
Contributor Author

uweseimet commented Oct 1, 2022

@akuker Regarding the gpiobus changes, these fix SonarCloud issues. Just a few lines are changed. If this is an issue for you, for this PR we can revert the changes in the 'hal' folder to what we currently have in develop. On the other hand, except for a few more changed lines (currently in the feature_more_unit_tests branch) I am done with updating 'hal'. I just merged these pending changes into the PR for feature_memory_management.
So we have two alternatives now:

  1. Keep everything in 'hal' as it is right now for this PR, with no other changes planned from my side, because all changes I have planned are now present in this PR.
  2. Revert 'hal' to what is currently in develop. In this case I would like to ask you to apply the pending changes, i.e. the 'hal' diffs between develop and what is now in feature_memory_management, as part of your upcoming changes.

I hope it's clear what I mean :). Of course I prefer option 1, because these changes jsut a few lines and because they have already been tested by @rdmark and me.

@akuker Please remember that I would like to merge this PR myself after approval, in order to clean up the merge message.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Oct 1, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 7 Code Smells

27.0% 27.0% Coverage
2.0% 2.0% Duplication

@akuker
Copy link
Copy Markdown
Member

akuker commented Oct 1, 2022

I hope it's clear what I mean :). Of course I prefer option 1, because these changes jsut a few lines and because they have already been tested by @rdmark and me.

Option 1 is good for me :-) The changes should be easy enough to merge in to my stuff. Have a great weekend!

@uweseimet
Copy link
Copy Markdown
Contributor Author

@akuker Once more, thank you for taking time to review my numerous changes. Have a great weekend as well!

@uweseimet uweseimet merged commit 255a6e1 into develop Oct 1, 2022
@uweseimet uweseimet deleted the feature_memory_management branch October 1, 2022 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Get rid of hard-coded/static transfer buffer size

2 participants