Skip to content

Update to c++17#385

Open
Ohisemega wants to merge 7 commits intoodin-detector:masterfrom
Ohisemega:update_to_c++17
Open

Update to c++17#385
Ohisemega wants to merge 7 commits intoodin-detector:masterfrom
Ohisemega:update_to_c++17

Conversation

@Ohisemega
Copy link
Contributor

Update Codebase to C++17 Standard

Remove boost::shared_ptr, boost::optional, boost::bind, boost::mutex, boost::to_string, boost::lexical_caststd::string, boost::function, etc
Replace the above boost libraries with std:: equivalent.
Update the Rapidjson library to version 1.1.0

@GDYendell
Copy link
Collaborator

Could you reword the first commit so that "Update the Rapidjson libary to version 1.1.0" is in the subject instead of the body? Let me know if you need help with this.

@Ohisemega
Copy link
Contributor Author

Hi Gary

Could you reword the first commit so that "Update the Rapidjson library to version 1.1.0" is in the subject instead of the body? Let me know if you need help with this.

Yes, I've done this!

@Ohisemega Ohisemega force-pushed the update_to_c++17 branch 2 times, most recently from 5286d43 to 98c65e2 Compare March 7, 2025 11:52
Ohisemega added 3 commits July 1, 2025 18:25
Update the Rapidjson library to version 1.1.0
Remove boost::shared_ptr, boost::optional, boost::bind, boost::mutex, boost::to_string, boost::lexical_cast<std::string>, boost::function, etc
Replace the above boost libraries with std equivalent
Add Move constructor and assignment operator to:
	1. Frame class
	2. FrameMetaData class
Replace push_back with emplace_back in scenarios where rvalue moves are feasible.
Use move semantics to elid construction of tempoary objects.
Ohisemega added 3 commits July 1, 2025 18:33
Add return value for move and copy constructor of FrameMetaData class
Change instance of boost::shared_ptr to std::shared_ptr
Replace FrameProcessorController's "static const std::string" members with "static const std::string_view"
Reimplement the IpcMessage's  get_param(const std::string param_name) to get_param(const std::string_view param_name)
Reimplement the IpcMessage's  has_param(const std::string param_name) to has_param(const std::string_view param_name)
Reimplement the IpcMessage's  internal_set_param(const std::string param_name, T const& param_value) to internal_set_param(const std::string_view param_name, T const& param_value)
Change OdinDataDefault const std::string to const std::string_view
Change FrameReceiver configuration string from const std::string to const std::string_view
Reformat set_param(const std::string param_name, T const& param_value) to set_param(const std::string_view param_name, T const& param_value)
Modify FrameReceiverConfig.h to convert
@ajgdls
Copy link
Contributor

ajgdls commented Aug 29, 2025

Hi @Ohisemega , to make this easier to progress it would be better if we can split it into multiple smaller PRs which can then be reviewed and potentially merged.
To start with, it would be nice to have a PR that lets the current odin-data source build under c++17 without any compiler warnings. Would you consider creating a new PR that just resolves that issue and then we could take a look at it?
Thanks

- Return instatiated std::shared_ptr from std::shared_ptr<BaseClass> maker()
- Change IpcMessage methods to take const std::string_view&
- In ParamContainer.h remove std::move() call on val
- Remove 'acquisition_ID_ = frame.acquisition_ID_' from FrameMetaData's
move-constructor
@Ohisemega Ohisemega self-assigned this Sep 15, 2025
@Ohisemega Ohisemega added enhancement Awaiting review/approval Has been addressed in a PR and needs to be approved or commented for rework by a reviewer labels Sep 15, 2025
@Ohisemega Ohisemega added this to the in-progress milestone Sep 15, 2025
@Ohisemega
Copy link
Contributor Author

Hi @Ohisemega , to make this easier to progress it would be better if we can split it into multiple smaller PRs which can then be reviewed and potentially merged. To start with, it would be nice to have a PR that lets the current odin-data source build under c++17 without any compiler warnings. Would you consider creating a new PR that just resolves that issue and then we could take a look at it? Thanks

Hi Alan!

Pardon my late response to this message @ajgdls . I would create a new issue in that case and open a new PR.

@Ohisemega Ohisemega linked an issue Nov 15, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Awaiting review/approval Has been addressed in a PR and needs to be approved or commented for rework by a reviewer enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants