-
Notifications
You must be signed in to change notification settings - Fork 399
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
iox-#2330 add notify systemd #2334
Changes from all commits
fe5015f
25155df
e09617a
76dd33f
485bfa4
d72b763
07c89d9
425db5e
952e91f
7abf543
bc13c4b
4aee645
864d7b1
a5f4bb4
4b4651c
3c334a6
8173252
efe3917
4ba2e9e
03d6f0d
62ba7b0
f72cc56
92fa972
2defac8
19f09c5
ad57509
b4e3db2
8b50b7b
5abaede
1d1b73b
93a1ac9
022d7fd
5f5afa1
ef4bffd
f73c9c6
41b10a2
ee418c3
2914009
402a626
250b4e4
7d6d7e2
d84998f
7ab593c
ab28d4f
58c5feb
f4cdef7
e96b661
20793f5
ef310cb
a5f4534
c9236aa
9775883
a60d701
ddce6b5
0921e43
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ | |
#define IOX_POSH_ROUDI_ROUDI_MULTI_PROCESS_HPP | ||
|
||
#include "iceoryx_platform/file.hpp" | ||
#include "iceoryx_platform/stdlib.hpp" | ||
#include "iceoryx_posh/iceoryx_posh_types.hpp" | ||
#include "iceoryx_posh/internal/roudi/introspection/mempool_introspection.hpp" | ||
#include "iceoryx_posh/internal/roudi/process_manager.hpp" | ||
|
@@ -33,15 +34,206 @@ | |
#include "iox/scope_guard.hpp" | ||
#include "iox/smart_lock.hpp" | ||
|
||
|
||
#include <condition_variable> | ||
#include <cstdint> | ||
#include <thread> | ||
|
||
#ifdef USE_SYSTEMD | ||
#include <systemd/sd-daemon.h> | ||
#endif | ||
|
||
#ifdef USE_SYSTEMD | ||
namespace iox::roudi::service_management | ||
{ | ||
class ServiceManagementSystemd; | ||
} // namespace iox::roudi::service_management | ||
using SendMessageServiceManagement = iox::roudi::service_management::ServiceManagementSystemd; | ||
#else | ||
namespace iox::roudi::service_management | ||
{ | ||
class NoServiceManagementSystemd; | ||
} // namespace iox::roudi::service_management | ||
using SendMessageServiceManagement = iox::roudi::service_management::NoServiceManagementSystemd; | ||
#endif | ||
Comment on lines
+46
to
+58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think a separate namespace is necessary. We kind of overdid it in iceoryx1 with namespaces. Since there are only three classes, it can stay in the I hope you don't hat me but I had an idea for a cleaner design. In general I always think of static polymorphism instead of dynamic polymorphism, since we cannot use dynamic polymorphism for data structure in the shared memory. But wat you did with the |
||
|
||
namespace iox | ||
{ | ||
namespace roudi | ||
{ | ||
using namespace iox::units::duration_literals; | ||
|
||
namespace service_management | ||
{ | ||
/** | ||
* @brief Interface class for systemd service handling | ||
* | ||
**/ | ||
class ServiceManagement | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, we have the policy that in general each class needs to be in its own hpp and cpp file. Can you please move this into |
||
{ | ||
public: | ||
virtual ~ServiceManagement() = default; | ||
ServiceManagement(ServiceManagement const& other) = delete; | ||
ServiceManagement(ServiceManagement&& other) = default; | ||
ServiceManagement& operator=(ServiceManagement const& other) = delete; | ||
ServiceManagement& operator=(ServiceManagement&& other) = default; | ||
|
||
static constexpr const uint16_t SIZE_STRING = 4096; ///< maximum size of string // 2 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pleas be more precise. The size of what string is this? |
||
static constexpr const uint8_t SIZE_THREAD_NAME = 15; ///< max size for thread name // 1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please just use |
||
|
||
|
||
virtual void processNotify() = 0; /// dbus signal handler | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please make the functions |
||
virtual void shutdown() = 0; /// Sets a shutdown flag | ||
virtual bool setThreadNameHelper(iox::string<SIZE_THREAD_NAME>& threadName) = 0; /// Sets a thread name | ||
virtual std::string getEnvironmentVariable(const char* const env_var) = 0; /// Get environment variable | ||
virtual bool sendSDNotifySignalHelper(const std::string_view state) = 0; /// Send notify | ||
Comment on lines
+85
to
+89
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do these functions need to be public, except for testing purposes? |
||
|
||
protected: | ||
ServiceManagement() = default; | ||
}; | ||
Comment on lines
+72
to
+93
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we could use the factory method pattern to get to our concrete implementation. Together with the moving away from the abstract base class to just a virtual base class, this would also eliminate the need for the This class would then also just hast a static |
||
|
||
/** | ||
* @brief Class to handle systemd service notifications | ||
* | ||
**/ | ||
class ServiceManagementSystemd final : public ServiceManagement | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please also move this to a separate hpp and cpp file. |
||
{ | ||
private: | ||
std::condition_variable watchdogNotifyCondition; ///< watch dog notification condition // 48 | ||
std::mutex watchdogMutex; ///< watch dog mutex // 40 | ||
std::thread m_listenThreadWatchdog; ///< thread that listens to systemd watchdog signals // 8 | ||
std::atomic_bool m_shutdown{false}; ///< indicates if service is being shutdown // 1 | ||
|
||
public: | ||
ServiceManagementSystemd() = default; | ||
ServiceManagementSystemd(ServiceManagementSystemd const& other) = delete; | ||
ServiceManagementSystemd(ServiceManagementSystemd&& other) = delete; | ||
ServiceManagementSystemd& operator=(ServiceManagementSystemd const& other) = delete; | ||
ServiceManagementSystemd& operator=(ServiceManagementSystemd&& other) = delete; | ||
|
||
/** | ||
* @brief Destructor joins the listenThreadWatchdog if it is still joinable, to ensure a proper termination | ||
**/ | ||
~ServiceManagementSystemd() final; | ||
|
||
/** | ||
* @brief Sets the shutdown flag to true, causing the systemd handler to stop. | ||
**/ | ||
void shutdown() final; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please always use |
||
|
||
/** | ||
* @brief Fetch required environment variable as a string | ||
* @param env_var Pointer to environment variable | ||
* @return Environment variable as std::string | ||
**/ | ||
std::string getEnvironmentVariable(const char* const env_var) final; | ||
|
||
/** | ||
* @brief Helper function to set thread name | ||
* @param threadName Thread name to be set | ||
* @return True if successfully set, otherwise false | ||
**/ | ||
bool setThreadNameHelper(iox::string<SIZE_THREAD_NAME>& threadName) final; | ||
|
||
/** | ||
* @brief Helper function to send SDNotify signals | ||
* @param state SDNotify state to be sent | ||
* @return True if signal sending is successful, otherwise false | ||
**/ | ||
#ifdef USE_SYSTEMD | ||
bool sendSDNotifySignalHelper(const std::string_view state) final | ||
{ | ||
auto result = IOX_POSIX_CALL(sd_notify)(0, state.data()).successReturnValue(1).evaluate(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In general we abstract these calls away in the
The |
||
if (result.has_error()) | ||
{ | ||
IOX_LOG(ERROR, | ||
"Failed to send " << state.data() | ||
<< " signal. Error: " << result.get_error().getHumanReadableErrnum()); | ||
return false; | ||
} | ||
return true; | ||
} | ||
#else | ||
bool sendSDNotifySignalHelper([[maybe_unused]] const std::string_view state) final | ||
{ | ||
// empty implementation | ||
return true; | ||
} | ||
#endif | ||
/** | ||
* @brief Function to manage the watchdog loop | ||
**/ | ||
void watchdogLoopHelper(); | ||
|
||
/** | ||
* @brief Method to process systemd notification logic | ||
**/ | ||
void processNotify() final; | ||
}; | ||
|
||
/** | ||
* @brief Empty implementation handler for non-systemd systems | ||
* | ||
**/ | ||
class NoServiceManagementSystemd final : public ServiceManagement | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be removed when the |
||
{ | ||
public: | ||
NoServiceManagementSystemd() = default; | ||
NoServiceManagementSystemd(NoServiceManagementSystemd const& other) = delete; | ||
NoServiceManagementSystemd(NoServiceManagementSystemd&& other) = default; | ||
NoServiceManagementSystemd& operator=(NoServiceManagementSystemd const& other) = delete; | ||
NoServiceManagementSystemd& operator=(NoServiceManagementSystemd&& other) = default; | ||
|
||
/** | ||
* @brief Empty implementation of destructor | ||
**/ | ||
~NoServiceManagementSystemd() final = default; | ||
|
||
/** | ||
* @brief Empty implementation of processNotify | ||
**/ | ||
void processNotify() final | ||
{ | ||
// empty implementation | ||
} | ||
|
||
/** | ||
* @brief Empty implementation of shutdown | ||
**/ | ||
void shutdown() final | ||
{ | ||
// empty implementation | ||
} | ||
|
||
/** | ||
* @brief Empty implementation of get environment variable | ||
**/ | ||
std::string getEnvironmentVariable([[maybe_unused]] const char* const env_var) final | ||
{ | ||
// empty implementation | ||
return "no implement"; | ||
} | ||
|
||
/** | ||
* @brief Empty implementation set thread name | ||
**/ | ||
bool setThreadNameHelper([[maybe_unused]] iox::string<SIZE_THREAD_NAME>& threadName) final | ||
{ | ||
// empty implementation | ||
return true; | ||
} | ||
|
||
/** | ||
* @brief Empty implementation send SDNotify signals | ||
**/ | ||
bool sendSDNotifySignalHelper([[maybe_unused]] const std::string_view state) final | ||
{ | ||
// empty implementation | ||
return true; | ||
} | ||
}; | ||
} // namespace service_management | ||
|
||
class RouDi | ||
{ | ||
public: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here. Please look how it is done with
IOX_EXPERIMENTAL_POSH_FLAG