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.

Sign up for GitHub

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

Jump to bottom

Support C++ versions before C++17 with boost::filesystem #177

Merged
merged 27 commits into from
Sep 28, 2023
Merged

Support C++ versions before C++17 with boost::filesystem #177

merged 27 commits into from
Sep 28, 2023

Conversation

bWF0dGhpYXMK
Copy link
Contributor

@bWF0dGhpYXMK bWF0dGhpYXMK commented Sep 18, 2023 •
edited by Dominik-K
Loading

Supporting C++-versions older than C++17. See comment below for background.

@bWF0dGhpYXMK
Copy link
Contributor Author

Please close #169 ( I hope that works )

For the moment, we have only support for BOOST_FILESYSTEM_VERSION 3. We make use of an SDK, which is in the moment not subject of change (HW support issues with later SDKs). I understand the point, as adding this change into mainline path, affects, many files and includes the start of having #defines inside the code and the CMakefile modification.

Without the pull request, the files have to be changed, after each release update to make use of libocpp with our SDK.
For the moment, I have not found a good way to add changes into the libocpp which are not part of the clone.

Summary: I have no real good argument, only that it is simpler to be follow libocpp updates with our SDK, without hand patching to many files.

Modification: The #ifndef switches good be placed into a header file, and this header file being included, to increase the readability.

@Dominik-K
Copy link
Contributor

Dominik-K commented Sep 19, 2023 •
edited
Loading

@bWF0dGhpYXMK

Modification: The #ifndef switches good be placed into a header file, and this header file being included, to increase the readability.

That would be really nice to avoid a lot of duplication. I suggest to name the file support_older_c++-versions.hpp and store it in ocpp/common.

Perhaps we might move this to a common repo soon. See related

@Dominik-K
Copy link
Contributor

Please close #169 ( I hope that works )

I think you should be able to do it as well (as PR-creator). At least only you can delete the branch in your fork.

Dominik-K
Dominik-K reviewed Sep 19, 2023
View reviewed changes
lib/CMakeLists.txt Outdated Show resolved Hide resolved
include/ocpp/common/pki_handler.hpp Outdated Show resolved Hide resolved
Dominik-K
Dominik-K reviewed Sep 19, 2023
View reviewed changes
CMakeLists.txt Outdated Show resolved Hide resolved
@Dominik-K Dominik-K changed the title Boost filesystem support signed Support older C++ versions with boost::filesystem Sep 20, 2023
Dominik-K
Dominik-K reviewed Sep 20, 2023
View reviewed changes
include/ocpp/common/sqlite_statement.hpp Outdated Show resolved Hide resolved
include/ocpp/common/support_older_c++_versions.hpp Outdated Show resolved Hide resolved
include/ocpp/common/support_older_c++_versions.hpp Outdated Show resolved Hide resolved
src/charge_point.cpp Outdated Show resolved Hide resolved
lib/ocpp/v16/charge_point_impl.cpp Outdated Show resolved Hide resolved
Dominik-K and others added 12 commits September 20, 2023 17:26
@Dominik-K
lib: add find_package(Boost)
a7de53e 
`cmake` failed with

CMake Error at lib/CMakeLists.txt:94 (target_link_libraries):
  Target "ocpp" links to:

    Boost::filesystem

  but the target was not found.  Possible reasons include:

    * There is a typo in the target name.
    * A find_package call is missing for an IMPORTED target.
    * An ALIAS target is missing.

Signed-off-by: Dominik K <[email protected]>
@Dominik-K
clang-format fixes
eff3cb8 
Signed-off-by: Dominik K <[email protected]>
@Dominik-K
clang-format 16.0.0 (from Docker-image in GitHub Action)
6bf3cbf 
Signed-off-by: Dominik K <[email protected]>
@Matthias-NIDEC
Set SDPX format and changed define selection via option
9faaac0 
Signed-off-by: Matthias Suess <[email protected]>
@Matthias-NIDEC
Fixed header ending
b20b170 
Signed-off-by: Matthias Suess <[email protected]>
@Dominik-K
cmake: re-add BOOSTFILESYSTEM
6d012c2 
Signed-off-by: Dominik K <[email protected]>
@Dominik-K
CP implementation: un-doubled this->logging
6ab07ee 
Signed-off-by: Dominik K <[email protected]>
@Dominik-K
CMakeLists.txt: space-indents instead tabs
67890d5 
Signed-off-by: Dominik K <[email protected]>
@Dominik-K
CP implementation: one message_log_path initialisation
f334351 
Signed-off-by: Dominik K <[email protected]>
@Dominik-K
cmake: verbose message for troubleshooting
ccf1c58 
Signed-off-by: Dominik K <[email protected]>
@Dominik-K
clang-format fix
efac291 
Signed-off-by: Dominik K <[email protected]>
@Dominik-K
charge_point.cpp: removed obsolete #include
0060a3e 
Signed-off-by: Dominik K <[email protected]>
Dominik-K
Dominik-K approved these changes Sep 25, 2023
View reviewed changes
Copy link
Contributor

@Dominik-K Dominik-K left a comment

Choose a reason for hiding this comment

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

LGTM

@bWF0dGhpYXMK Can you check if it's still working on your side, please?
@hikinggrass Please review.

@Dominik-K Dominik-K self-assigned this Sep 25, 2023
@bWF0dGhpYXMK
Copy link
Contributor Author

LGTM

@bWF0dGhpYXMK Can you check if it's still working on your side, please? @hikinggrass Please review.

Thank you, i recompiled and worked here using boost::filesystem.

hikinggrass
hikinggrass requested changes Sep 25, 2023
View reviewed changes
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
lib/CMakeLists.txt Outdated Show resolved Hide resolved
include/ocpp/common/support_older_c++_versions.hpp Outdated Show resolved Hide resolved
include/ocpp/common/support_older_c++_versions.hpp Outdated Show resolved Hide resolved
include/ocpp/common/sqlite_statement.hpp Outdated Show resolved Hide resolved
Dominik-K
Dominik-K approved these changes Sep 28, 2023
View reviewed changes
Copy link
Contributor

@Dominik-K Dominik-K left a comment

Choose a reason for hiding this comment

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

LGTM

@hikinggrass hikinggrass merged commit 9c445db into EVerest:main Sep 28, 2023
3 checks passed
@Dominik-K Dominik-K mentioned this pull request Nov 13, 2023
Open
3 tasks
@Dominik-K Dominik-K changed the title Support older C++ versions with boost::filesystem Support C++ versions before C++17 with boost::filesystem Nov 13, 2023
@Dominik-K Dominik-K mentioned this pull request Dec 11, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Pietfried Pietfried Pietfried approved these changes

@Dominik-K Dominik-K Dominik-K approved these changes

@hikinggrass hikinggrass hikinggrass approved these changes

Assignees

@Dominik-K Dominik-K

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@bWF0dGhpYXMK @Dominik-K @hikinggrass @Pietfried @Matthias-NIDEC