iox-#2330 add notify systemd #2334

khromenokroman · 2024-08-22T14:58:44Z

Notes for Reviewer

Pre-Review Checklist for the PR Author

Checklist for the PR Reviewer

Consider a second reviewer for complex new features or larger refactorings
Commits are properly organized and messages are according to the guideline
Code according to our coding style and naming conventions
Unit tests have been written for new behavior
Public API changes are documented via doxygen
Copyright owner are updated in the changed files
All touched (C/C++) source code files from iceoryx_hoofs have been added to ./clang-tidy-diff-scans.txt
PR title describes the changes

Post-review Checklist for the PR Author

All open points are addressed and tracked via issues

References

Closes TBD

@elBoberido sorry if it turned out very badly, I still don't quite understand how your application works, but there is a suggestion to add a check, if the application was launched without using systemd, then everything will remain that way if not, then I will send a ready signal when I am ready to receive clients

this is a continuation based on your answer #2330

are you interested in this functionality? I would really like it to be added, I need it :)

elBoberido · 2024-08-22T18:57:58Z

@khromenokroman yes, the functionality would be appreciated.

Can you add a cmake switch which is off by default to not add a hard dependency to systemd? You can check how it is done with IOX_EXPERIMENTAL_POSH_FLAG and replicate it.

Furthermore, if you install the git commit hooks, it takes care of setting the issue number for commits.
https://github.com/eclipse-iceoryx/iceoryx/blob/main/tools/git-hooks/Readme.md
Currently there are commits without the issue number.

Another issue will be to integrate the systemd support so that it does not affect the build of our platforms. In general we try to abstract this away and then have a specific implementations for platforms which do not support a specific feature. This is done for the IpcChannel which is using NamedPipe on Windows and UnixDomainSocket on the other platforms. You can have a look at iceoryx_posh/include/iceoryx_posh/internal/runtime/ipc_interface_base.hpp. Ideally this would not be done with and ifdef but in a similar manner like in iceoryx_platform but we did not yet implement a similar structure for iceoryx_posh yet.

So there is quite a lot to do in order to get this upstream :)

This change updates the GitHub Action to include the libsystemd-dev package in the list of dependencies. Adding this package ensures that all necessary development libraries are available for builds.

Introduce a configurable option to enable systemd support in the build process. This helps in integrating systemd features if needed while keeping it optional by default.

elBoberido · 2024-08-22T21:36:23Z

iceoryx_platform/cmake/IceoryxPackageHelper.cmake

@@ -207,7 +207,7 @@ Macro(iox_add_executable)
 if ( QNX )
 target_link_libraries(${IOX_TARGET} ${IOX_LIBS_QNX})
 elseif ( LINUX )
- target_link_libraries(${IOX_TARGET} ${IOX_LIBS_LINUX})
+ target_link_libraries(${IOX_TARGET} ${IOX_LIBS_LINUX} systemd)


Please add this only to the iox-roudi target

@elBoberido fixed

elBoberido · 2024-08-22T21:38:07Z

iceoryx_posh/source/roudi/roudi.cpp

+ IOX_LOG(INFO, "Start watchdog");
+ while (m_runHandleRuntimeMessageThread.load())
+ {
+ if (auto wdres = sd_notify(0, "WATCHDOG=1") < 0)


Please use the IOX_POSIX_CALL. It takes care of the error handling

Please use the IOX_POSIX_CALL. It takes care of the error handling

@elBoberido fixed

elBoberido · 2024-08-22T21:38:27Z

iceoryx_posh/include/iceoryx_posh/internal/roudi/roudi.hpp

@@ -129,6 +132,7 @@ class RouDi
 private:
 std::thread m_monitoringAndDiscoveryThread;
 std::thread m_handleRuntimeMessageThread;
+ std::thread listen_thread_watchdog; // 8


Please prefix members with m_

@elBoberido I'm sorry for my mistake, it's been fixed

No Problem. But do not use ifdefs in the code. Please write an abstraction, similar to what we did IpcChannelType and use that abstraction. For platforms which do not support systemd, write an alternative which basically does nothing when the methods are called. Having ifdefs all over the place makes it hard to maintain and therefore we choose the other approach.

@elBoberido I'm not sure if I understood you correctly, please look at the roudi.hpp file did you talk about this? The announcement and the definition will share this is a test for now, thank you :)

elBoberido · 2024-08-22T21:40:30Z

iceoryx_posh/source/roudi/roudi.cpp

+ * We get information about how they are running. If as a unit, then we launch
+ * watchdog and send a notification about the launch, otherwise we do nothing
+ */
+ const char* invocation_id = std::getenv("INVOCATION_ID");


Please use iox_getenv_s

Please use iox_getenv_s

@elBoberido fixed

elBoberido · 2024-08-22T21:44:56Z

iceoryx_posh/source/roudi/roudi.cpp

+ if (pthread_setname_np(listen_thread_watchdog.native_handle(), "watchdog") != 0)
+ {
+ std::array<char, SIZE_ERROR_MESSAGE> buf{};
+ strerror_r(errno, buf.data(), buf.size());
+ IOX_LOG(ERROR, "Can not set name for thread watchdog: " << std::string(buf.data()));
+ }


Please use setThreadName

Please use setThreadName

@elBoberido fixed

elBoberido · 2024-08-22T21:48:00Z

iceoryx_posh/source/roudi/roudi.cpp

+ IOX_LOG(ERROR, "WatchDogError: " << std::string(buf.data()));
+ return;
+ }
+ std::this_thread::sleep_for(std::chrono::seconds(1));


This needs to be replaced with a blocking timed_wait on a semaphore or something else which can be interupted, else the test time will be massively increased.

@elBoberido I'm not very good at multithreaded programming, but in my opinion it's not a very elegant solution :)

@elBoberido fixed

Introduce preprocessor directives to conditionally enable systemd-related code and linking. This change allows for builds without systemd on Linux by checking for the USE_SYSTEMD flag, offering greater flexibility in different environments.

…read_watchdog Standardize the naming convention for the listen thread watchdog variable to match the member variable naming pattern. This change ensures consistency and improves code readability.

Refactored thread naming and error handling for the watchdog thread. Replaced raw `sd_notify` usage with `IOX_POSIX_CALL` to better handle errors. Enhanced log messages for clearer error reporting.

In the `roudi.cpp` file, added early returns to handle errors more effectively when setting the thread watchdog name and sending READY=1 or WATCHDOG=1 signals. This ensures the system will not proceed if critical setup steps fail, enhancing reliability and maintainability.

…INVOCATION_ID' Replaced getenv with iox_getenv_s to fetch 'INVOCATION_ID' securely and handle errors. Added necessary includes and modified related variable and constant for better clarity.

…h_roudi Update the CMake script to enable systemd support only for iceoryx_posh_roudi. This ensures that systemd is correctly utilized where needed and prevents potential misconfigurations for other targets.

…tion The 'libsystemd-dev' package was removed from the install script in the 'install-iceoryx-deps-and-clang' GitHub action. This change reduces the set of dependencies to only those required for successful execution.

…lass Extracted systemd-related functionality into a new Systemd_service_handler class. This refactor improves modularity and simplifies the main RouDi code by encapsulating systemd-specific logic, making it easier to maintain and extend in the future.

Implemented a shutdown method in Systemd_service_handler to allow graceful termination of the watchdog thread. This enhancement ensures proper resource cleanup and improves system robustness by avoiding potential hang-ups during the shutdown process.

…mpilation Moved the Systemd include directive under a conditional compilation block using the USE_SYSTEMD flag. This change ensures that the dependency on Systemd is only included when necessary, reducing potential compilation issues and improving portability.

…nsistency Renamed class names and method names to follow camel case conventions, improving code readability and consistency. Updated related variables and comments to match the new naming scheme.

Updated `roudi_systemd` to `roudiSystemd` and `process_notify` to `processNotify` for consistency with naming conventions. This enhances code readability and maintains consistent style throughout the file.

…guide Include an example unit file for enabling systemd support with the `-DUSE_SYSTEMD=ON` build option. This addition clarifies how to configure and manage the service using systemd.

Introduce std::condition_variable and std::mutex to replace sleep logic in the watchdog loop. This change aims to improve resource management and responsiveness to shutdown signals, making the watchdog more efficient.

Extract environment variable retrieval logic into a static method. This refactoring improves code readability and reuse, ensuring cleaner and more maintainable code.

…adability Improve SystemdServiceHandler readability by adding detailed comments and helper functions. This refactor helps in managing the watchdog loop and sending notifications more clearly, ensuring proper thread name setting and signal handling.

…re handling Enhanced the build_systemd.yaml to include steps for better error handling and visibility. Added a 30-second wait and refined the logic to check the status of the roudi service, displaying journal logs if it fails to start. This improves debugging capability in the CI pipeline.

khromenokroman · 2024-08-28T20:47:16Z

Another issue is testing. We need to ensure that the suystemd support does not break. This could be done by having a CI target with systemd and maybe also an integration test. I'm not sure how difficult it would be to create such an integration test, though.

@elBoberido I did a test in ci, but I can't figure out where I need to add it to make it work for you, I added it here build-test.yml but your ci didn't run my ci :( , I've temporarily done it here build_systemd.yaml

https://github.com/khromenokroman/iceoryx/actions/runs/10604085974/job/29389869104

Introduced a new job in build-test.yml to build and run roudi with systemd on Ubuntu. This includes error handling steps such as service status checks, logging, and a wait period to enhance reliability and debugging for the CI pipeline.

elBoberido · 2024-08-29T13:49:05Z

@khromenokroman for new contributors, the github CI needs to be triggered by a committer for their first PR. This is the policy for Eclipse projects so we cannot really do something here, except if you create a small PR which is merged first. Then the github CI would also be triggered with each push.

If you like, there is #2209. It should be done if a few minutes but the creator of the issue did not respond anymore. You could create a small PR and move using iox_ssize_t = int; from iceoryx_platform/win/include/iceoryx_platform/unistd.hpp to iceoryx_platform/win/include/iceoryx_platform/types.hpp and replace the using ssize_t = size_t; from the types.hpp. There are then a few other file in iceoryx_platform/win where ssize_t needs to be replaced with iox_ssize_t but that's minimal. What do you think? It would make the handling of this PR simpler for you.

…ace typo Deleted the build_systemd.yaml GitHub Actions workflow file. Corrected a namespace typo from `systemd` to `service_management` in the roudi.hpp file.

khromenokroman · 2024-08-29T14:42:38Z

for new contributors, the github CI needs to be triggered by a committer for their first PR. This is the policy for Eclipse projects so we cannot really do something here, except if you create a small PR which is merged first. Then the github CI would also be triggered with each push.

@elBoberido no problem, add ci in build-test.yml

khromenokroman · 2024-08-29T14:46:46Z

If you like, there is #2209. It should be done if a few minutes but the creator of the issue did not respond anymore. You could create a small PR and move using iox_ssize_t = int; from iceoryx_platform/win/include/iceoryx_platform/unistd.hpp to iceoryx_platform/win/include/iceoryx_platform/types.hpp and replace the using ssize_t = size_t; from the types.hpp. There are then a few other file in iceoryx_platform/win where ssize_t needs to be replaced with iox_ssize_t but that's minimal. What do you think? It would make the handling of this PR simpler for you.

@elBoberido I'm still new to your project, and I didn't understand anything :) let's do this PR, and I'll try to change what you want with another PR, or do you need to do it in this PR?

Cast `info.total_size` to unsigned int in the `iox_getenv_s` call to ensure type matching and eliminate potential issues with type mismatches. This change improves code robustness and ensures adherence to function signature requirements.

…y (rev.2) Cast `info.total_size` to unsigned int in the `iox_getenv_s` call to ensure type matching and eliminate potential issues with type mismatches. This change improves code robustness and ensures adherence to function signature requirements.

khromenokroman · 2024-08-30T06:49:16Z

@elBoberido I think that's the problem here
int iox_getenv_s(size_t* actual_size_with_null, char* buffer, size_t buffer_capacity, const char* name)
size_t buffer_capacity but

struct BufferInfo
{
    uint64_t used_size{0};
    uint64_t total_size{0};
};

uint64_t total_size{0}; ??

in build 32bit ubuntu, get error:

/home/runner/work/iceoryx/iceoryx/iceoryx_posh/source/roudi/roudi.cpp:608:26:   required from here
/home/runner/work/iceoryx/iceoryx/iceoryx_posh/source/roudi/roudi.cpp:610:86: error: conversion from ‘uint64_t’ {aka ‘long long unsigned int’} to ‘unsigned int’ may change value [-Werror=conversion]
  610 |         auto result = IOX_POSIX_CALL(iox_getenv_s)(&actualSizeWithNull, buffer, info.total_size, env_var)
      |                                                                                 ~~~~~^~~~~~~~~~
[ 34%] Building CXX object hoofs/test/CMakeFiles/hoofs_moduletests.dir/moduletests/test_container_uninitialized_array.cpp.o
cc1plus: all warnings being treated as errors
gmake[2]: *** [posh/CMakeFiles/iceoryx_posh_roudi.dir/build.make:258: posh/CMakeFiles/iceoryx_posh_roudi.dir/source/roudi/roudi.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:1300: posh/CMakeFiles/iceoryx_posh_roudi.dir/all] Error 2
gmake[1]: *** Waiting for unfinished jobs....

…_notify_systemd

khromenokroman · 2024-09-01T12:37:20Z

@elBoberido Is everything okay, are we doing a merge request?

khromenokroman · 2024-09-02T15:11:54Z

@elBoberido added some tests

elBoberido

I did not yet look at the test but there are already a few changes you could implement.

Oh, is it possible to squash some of the commits?

elBoberido · 2024-09-02T17:19:01Z

.github/workflows/build-test.yml

+ - name: Update
+ run: sudo apt update
+ - name: Install depends
+ run: sudo apt install -y gcc g++ cmake libacl1-dev libncurses5-dev pkg-config libsystemd-dev
+ - name: Checkout
+ uses: actions/checkout@v4


Please use this like in other tasks

- name: Install iceoryx dependencies and clang-tidy uses: ./.github/actions/install-iceoryx-deps-and-clang

Only libsystemd-dev would be installed here.

@elBoberido fixed

elBoberido · 2024-09-02T17:27:45Z

.github/workflows/build-test.yml

+ - name: Cmake cache
+ run: cmake -Bbuild -Hiceoryx_meta -DBUILD_SHARED_LIBS=ON -DUSE_SYSTEMD=ON -DBUILD_TEST=ON
+ - name: build
+ run: cmake --build build -j 16
+ - name: Install
+ run: sudo cmake --build build --target install


Please extend the tools/iceoryx_build_test.sh script and call it here with the appropriate arguments, e.g.

./tools/iceoryx_build_test.sh build-strict build-all out-of-tree build-shared test-add-user use-systemd

@elBoberido fixed

elBoberido · 2024-09-02T17:36:19Z

.github/workflows/build-test.yml

+ - name: Ldconfig run
+ run: sudo ldconfig
+ - name: Create unit file
+ run: echo -e "[Unit]\nDescription=Test application roudi\n\n[Service]\nType=notify\nRestartSec=10\nRestart=always\nExecStart=/usr/local/bin/iox-roudi\nTimeoutStartSec=10\nWatchdogSec=5\n\n[Install]\nWantedBy=multi-user.target" | sudo tee /usr/lib/systemd/system/test_iox.service > /dev/null
+ - name: Show unit
+ run: cat /usr/lib/systemd/system/test_iox.service
+ - name: Daemon reload
+ run: sudo systemctl daemon-reload
+ - name: Check status
+ run: sudo systemctl status test_iox || true
+ - name: Start roudi
+ run: |
+ sudo systemctl start test_iox || (echo "Failed to start service"; sudo journalctl -u test_iox -n 50; exit 1)
+ - name: Wait for 30 seconds
+ run: sleep 30
+ - name: Check roudi
+ run: sudo systemctl status test_iox || (echo "Failed to start service"; sudo journalctl -u test_iox -n 50; exit 1)
+ - name: Stop roudi
+ run: sudo systemctl stop test_iox
+ - name: Show journal
+ run: sudo journalctl -u test_iox -n 100
+ - name: Start test (integration) posh with systemd
+ run: cd build/posh/test && ./posh_integrationtests
+ - name: Start test (module) posh with systemd
+ run: cd build/posh/test && ./posh_moduletests


Please move this to a bash script in tools/ci so that it can easily be tested locally. The tools/iceoryx_build_test.sh call and installation of libsystemd-dev could also be moved there. The script could be named build-test-ubuntu-systemd.sh

Please also add this to the beginning of the script

if [ "$USER" != "root" ]; then echo "Please run this as root or with sudo" exit 1 fi

and remove the sudo calls so that developer are aware that their system will be altered when they run the script.

@elBoberido fixed

elBoberido · 2024-09-02T17:43:22Z

iceoryx_platform/cmake/IceoryxPackageHelper.cmake

+ if(USE_SYSTEMD AND ("${IOX_TARGET}" STREQUAL "iceoryx_posh_roudi"))
+ message(STATUS "[i] Configuring ${IOX_TARGET} with systemd support.")
+ target_compile_definitions(${IOX_TARGET} PRIVATE USE_SYSTEMD=1)
+ target_link_libraries(${IOX_TARGET} PUBLIC ${IOX_PUBLIC_LIBS_LINUX} PRIVATE ${IOX_PRIVATE_LIBS_LINUX} systemd)
+ else()
+ target_link_libraries(${IOX_TARGET} PUBLIC ${IOX_PUBLIC_LIBS_LINUX} PRIVATE ${IOX_PRIVATE_LIBS_LINUX})
+ endif()


This is the wrong place. It needs to be done in iceoryx_posh/CMakeLists.txt. If possible added to PRIVATE_LIBS_LINUX with a cmake generator expression. If that is too cumbersome, it can be done with target_link_libraries(iceoryx_posh_roudi ...). I'm also wondering why target_link_libraries is called here, since it does not add systemd or something similar.

Furthermore, instead of the define, please have a look how it is done with IOX_EXPERIMENTAL_POSH_FLAG and replicate that. Else there might be issues when the roudi lib is used in out-of-tree builds in other binaries.

You also do not need to print the status since it should already be done via build_options.cmake

elBoberido · 2024-09-02T17:54:38Z

iceoryx_platform/cmake/IceoryxPackageHelper.cmake

+ if(USE_SYSTEMD AND ("${IOX_TARGET}" STREQUAL "posh_moduletests"))
+ target_compile_definitions(${IOX_TARGET} PRIVATE USE_SYSTEMD_TEST=1)
+ message(STATUS "[i] Configuring ${IOX_TARGET} with systemd support.")
+ endif ()


Same here. Please look how it is done with IOX_EXPERIMENTAL_POSH_FLAG

elBoberido · 2024-09-03T11:44:53Z

iceoryx_posh/source/roudi/roudi.cpp

+ SendMessageServiceManagement roudiSendMessage;
+ roudiSendMessage.processNotify();


Since processNotify needs to be called anyway, it should be done in the constructor.

elBoberido · 2024-09-03T11:52:58Z

iceoryx_posh/source/roudi/roudi.cpp

+ m_shutdown.store(true);
+}
+
+std::string ServiceManagementSystemd::getEnvironmentVariable(const char* const env_var)


Is this used for anything else than checking whether the INVOCATION_ID is set?

If not, then this should not be a virtual method but just an implementation detail of ServiceManagementSystemd and be more in line with e.g.

bool runUnderServiceManagement();

elBoberido · 2024-09-03T12:05:08Z

iceoryx_posh/source/roudi/roudi.cpp

+bool ServiceManagementSystemd::setThreadNameHelper(iox::string<SIZE_THREAD_NAME>& threadName)
+{
+ bool status_change_name = iox::setThreadName(threadName);
+ if (!status_change_name)
+ {
+ IOX_LOG(ERROR, "Cannot set name for thread " << threadName);
+ return false;
+ }
+ return true;
+}
+
+void ServiceManagementSystemd::watchdogLoopHelper()
+{
+ IOX_LOG(INFO, "Start watchdog");
+ while (!m_shutdown.load())
+ {
+ std::unique_lock<std::mutex> lock(watchdogMutex);
+ if (watchdogNotifyCondition.wait_for(lock, std::chrono::seconds(1), [this] { return m_shutdown.load(); }))
+ {
+ break;
+ }
+ if (!sendSDNotifySignalHelper("WATCHDOG=1"))
+ {
+ return;
+ }
+ }
+}


For this and also the shutdown, you could use a nice abstraction and do not need to take care of all the thread handling.

You can add this to the base class

PeriodicTask m_watchdog{ PeriodicTaskManualStart, "SvcMgmtWatchdog", *this, &ServiceManagement::notifyWatchdog};

In the ServiceManagement constructor you can than just do a

if (runUnderServiceManagement) { notifyProcessReady(); m_watchdog.start(units::Duration::from_secs(1)); // or just m_watchdog.start(1_s); if the string literals are included }

The task is automatically stopped when the dtor is called. You basically do not have to take care or anything else.

elBoberido · 2024-09-03T12:09:39Z

iceoryx_posh/source/roudi/roudi.cpp

+ }
+}
+
+void ServiceManagementSystemd::processNotify()


Also just nitpicking. For someone not that familiar with systemd, having this method called notifyProcessReady gives a better understanding what it does.

elBoberido · 2024-09-03T12:11:46Z

iceoryx_posh/source/roudi/roudi.cpp

+ IOX_LOG(WARN, "Run application in system management");
+
+ m_listenThreadWatchdog = std::thread([this] {
+ iox::string<SIZE_THREAD_NAME> nameThread = "Watchdog";
+ if (!setThreadNameHelper(nameThread) || !sendSDNotifySignalHelper("READY=1"))
+ {
+ return;
+ }
+
+ IOX_LOG(DEBUG, "Notify READY=1 successful");


With the proposals from above, this function would be reduced to just these lines and the the PeriodicTask would take care of the rest.

iox-eclipse-iceoryx#2330 Add libsystemd-dev to install dependencies

fe5015f

This change updates the GitHub Action to include the libsystemd-dev package in the list of dependencies. Adding this package ensures that all necessary development libraries are available for builds.

khromenokroman force-pushed the iox-#2330-add_notify_systemd branch from c88237d to fe5015f Compare August 22, 2024 20:24

iox-eclipse-iceoryx#2330 Add systemd build option

25155df

Introduce a configurable option to enable systemd support in the build process. This helps in integrating systemd features if needed while keeping it optional by default.

elBoberido reviewed Aug 22, 2024

View reviewed changes

khromenokroman added 11 commits August 23, 2024 00:09

iox-eclipse-iceoryx#2330 Rename listen_thread_watchdog to m_listen_th…

76dd33f

…read_watchdog Standardize the naming convention for the listen thread watchdog variable to match the member variable naming pattern. This change ensures consistency and improves code readability.

iox-eclipse-iceoryx#2330 Improve thread management and error logging

485bfa4

Refactored thread naming and error handling for the watchdog thread. Replaced raw `sd_notify` usage with `IOX_POSIX_CALL` to better handle errors. Enhanced log messages for clearer error reporting.

iox-eclipse-iceoryx#2330 Refactor environment variable handling for '…

07c89d9

…INVOCATION_ID' Replaced getenv with iox_getenv_s to fetch 'INVOCATION_ID' securely and handle errors. Added necessary includes and modified related variable and constant for better clarity.

iox-eclipse-iceoryx#2330 Refine systemd configuration for iceoryx_pos…

425db5e

…h_roudi Update the CMake script to enable systemd support only for iceoryx_posh_roudi. This ensures that systemd is correctly utilized where needed and prevents potential misconfigurations for other targets.

iox-eclipse-iceoryx#2330 Refactor class names and method names for co…

864d7b1

…nsistency Renamed class names and method names to follow camel case conventions, improving code readability and consistency. Updated related variables and comments to match the new naming scheme.

khromenokroman changed the title ~~Iox #2330 add notify systemd~~ Iox-#2330 add notify systemd Aug 24, 2024

khromenokroman changed the title ~~Iox-#2330 add notify systemd~~ iox-#2330 add notify systemd Aug 24, 2024

khromenokroman added 2 commits August 24, 2024 14:33

iox-eclipse-iceoryx#2330 Refactor systemd handler variable names

a5f4bb4

Updated `roudi_systemd` to `roudiSystemd` and `process_notify` to `processNotify` for consistency with naming conventions. This enhances code readability and maintains consistent style throughout the file.

iox-eclipse-iceoryx#2330 Update changelog (text, reference)

4b4651c

khromenokroman force-pushed the iox-#2330-add_notify_systemd branch from 0fadbad to 3c334a6 Compare August 24, 2024 13:07

khromenokroman added 4 commits August 24, 2024 15:07

iox-eclipse-iceoryx#2330 Add systemd support example to installation …

3c334a6

…guide Include an example unit file for enabling systemd support with the `-DUSE_SYSTEMD=ON` build option. This addition clarifies how to configure and manage the service using systemd.

iox-eclipse-iceoryx#2330 Add condition_variable to watchdog loop

8173252

Introduce std::condition_variable and std::mutex to replace sleep logic in the watchdog loop. This change aims to improve resource management and responsiveness to shutdown signals, making the watchdog more efficient.

iox-eclipse-iceoryx#2330 Refactor environment variable retrieval

efe3917

Extract environment variable retrieval logic into a static method. This refactoring improves code readability and reuse, ensuring cleaner and more maintainable code.

iox-eclipse-iceoryx#2330 Remove systemd build workflow and fix namesp…

e96b661

…ace typo Deleted the build_systemd.yaml GitHub Actions workflow file. Corrected a namespace typo from `systemd` to `service_management` in the roudi.hpp file.

khromenokroman added 2 commits August 29, 2024 18:21

Merge branch 'eclipse-iceoryx:main' into iox-eclipse-iceoryx#2330-add…

a5f4534

…_notify_systemd

khromenokroman requested a review from elBoberido September 2, 2024 15:11

elBoberido reviewed Sep 3, 2024

View reviewed changes

khromenokroman force-pushed the iox-#2330-add_notify_systemd branch 10 times, most recently from 510c6c4 to c9236aa Compare September 5, 2024 09:17

khromenokroman added 5 commits September 5, 2024 11:18

iox-eclipse-iceoryx#2330 Refactor ci

c9236aa

iox-eclipse-iceoryx#2330 Edit CMakeLists.txt

9775883

iox-eclipse-iceoryx#2330 Edit CMakeLists.txt

a60d701

iox-eclipse-iceoryx#2330 Edit note-release

ddce6b5

iox-eclipse-iceoryx#2330 Sort bazel

0921e43

		SendMessageServiceManagement roudiSendMessage;
		roudiSendMessage.processNotify();

iox-#2330 add notify systemd #2334

Are you sure you want to change the base?

iox-#2330 add notify systemd #2334

Conversation

khromenokroman commented Aug 22, 2024 • edited Loading

Notes for Reviewer

Pre-Review Checklist for the PR Author

Checklist for the PR Reviewer

Post-review Checklist for the PR Author

References

elBoberido commented Aug 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

khromenokroman commented Aug 28, 2024

elBoberido commented Aug 29, 2024

khromenokroman commented Aug 29, 2024

khromenokroman commented Aug 29, 2024

khromenokroman commented Aug 30, 2024 • edited Loading

khromenokroman commented Sep 1, 2024

khromenokroman commented Sep 2, 2024

elBoberido left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

khromenokroman commented Aug 22, 2024 •

edited

Loading

khromenokroman commented Aug 30, 2024 •

edited

Loading

elBoberido left a comment •

edited

Loading