Skip to content

Replace floating-point seconds with integer milliseconds in timing config #388

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

Open
shegazyy wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Replace floating-point seconds with integer milliseconds in timing config #388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
60a9f33
Replace floating-point seconds with integer milliseconds in config
shegazyy Jul 28, 2026
ef9f1f4
Address review: _ms suffixes, revert schema version, fix warning
shegazyy Aug 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
182 changes: 109 additions & 73 deletions score/launch_manager/src/daemon/src/configuration/details/flatbuffer_config_loader_UT.cpp
View file
Open in desktop

Large diffs are not rendered by default.

112 changes: 22 additions & 90 deletions score/launch_manager/src/daemon/src/configuration/details/flatbuffer_type_converters.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include <score/assert.hpp>
#include <sys/types.h>
#include <cstdint>
#include <limits>
#include <string>
#include <vector>

Expand All @@ -34,8 +33,9 @@ namespace
{

template <typename T>
score::cpp::expected<T, IConfigLoader::Error> requireScalarValue(const ::flatbuffers::Optional<T>& field,
const std::string_view field_name)
score::cpp::expected<T, IConfigLoader::Error> requireScalarValue(
const ::flatbuffers::Optional<T>& field,
const std::string_view field_name)
{
if (!field.has_value())
{
Expand All @@ -56,29 +56,6 @@ std::optional<T> optionalScalarValue(const ::flatbuffers::Optional<T>& field)
namespace details
{

constexpr double kSecondsToMilliseconds = 1000.0;

score::cpp::expected<uint32_t, IConfigLoader::Error> secondsToMs(double seconds)
{
if (seconds < 0.0)
{
LM_LOG_ERROR() << "Negative time value " << seconds << " seconds is not supported";
return score::cpp::make_unexpected(IConfigLoader::Error::InvalidFormat);
}
if (seconds * kSecondsToMilliseconds > static_cast<double>(std::numeric_limits<uint32_t>::max()))
{
LM_LOG_ERROR() << "Time value " << seconds << " seconds exceeds maximum representable milliseconds";
return score::cpp::make_unexpected(IConfigLoader::Error::InvalidFormat);
}
const auto result = static_cast<uint32_t>(seconds * kSecondsToMilliseconds);
if (seconds > 0.0 && result == 0U)
{
LM_LOG_ERROR() << "Sub-millisecond time value " << seconds << " seconds rounds to 0ms";
return score::cpp::make_unexpected(IConfigLoader::Error::InvalidFormat);
}
return result;
}

ApplicationType convertApplicationType(fb::ApplicationType fb_type)
{
switch (fb_type)
Expand Down Expand Up @@ -197,18 +174,13 @@ score::cpp::expected<std::optional<RestartAction>, IConfigLoader::Error> convert
{
return score::cpp::make_unexpected(number_of_attempts.error());
}
auto delay_before_restart = requireScalarValue(ra->delay_before_restart(), "RestartAction::delay_before_restart");
auto delay_before_restart =
requireScalarValue(ra->delay_before_restart_ms(), "RestartAction::delay_before_restart_ms");
if (!delay_before_restart.has_value())
{
return score::cpp::make_unexpected(delay_before_restart.error());
}
auto delay_ms = secondsToMs(*delay_before_restart);
if (!delay_ms.has_value())
{
LM_LOG_ERROR() << "Invalid value for RestartAction::delay_before_restart";
return score::cpp::make_unexpected(delay_ms.error());
}
return std::optional<RestartAction>{RestartAction{*number_of_attempts, *delay_ms}};
return std::optional<RestartAction>{RestartAction{*number_of_attempts, *delay_before_restart}};
}

std::optional<SwitchRunTargetAction> convertSwitchRunTargetAction(const fb::SwitchRunTargetAction* sa)
Expand Down Expand Up @@ -236,7 +208,7 @@ score::cpp::expected<ComponentAliveSupervision, IConfigLoader::Error> convertCom
if (fb_cas != nullptr)
{
auto reporting_cycle =
requireScalarValue(fb_cas->reporting_cycle(), "ComponentAliveSupervision::reporting_cycle");
requireScalarValue(fb_cas->reporting_cycle_ms(), "ComponentAliveSupervision::reporting_cycle_ms");
if (!reporting_cycle.has_value())
{
return score::cpp::make_unexpected(reporting_cycle.error());
Expand All @@ -247,13 +219,7 @@ score::cpp::expected<ComponentAliveSupervision, IConfigLoader::Error> convertCom
{
return score::cpp::make_unexpected(failed_cycles_tolerance.error());
}
auto reporting_cycle_ms = secondsToMs(*reporting_cycle);
if (!reporting_cycle_ms.has_value())
{
LM_LOG_ERROR() << "Invalid value for ComponentAliveSupervision::reporting_cycle";
return score::cpp::make_unexpected(reporting_cycle_ms.error());
}
result.reporting_cycle_ms = *reporting_cycle_ms;
result.reporting_cycle_ms = *reporting_cycle;
result.failed_cycles_tolerance = *failed_cycles_tolerance;
result.min_indications = optionalScalarValue(fb_cas->min_indications());
result.max_indications = optionalScalarValue(fb_cas->max_indications());
Expand Down Expand Up @@ -418,30 +384,19 @@ score::cpp::expected<DeploymentConfig, IConfigLoader::Error> convertDeploymentCo
"DeploymentConfig::working_dir must never be nullptr as it is required in the schema");
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(
fb_dc->sandbox(), "DeploymentConfig::sandbox must never be nullptr as it is required in the schema");
auto ready_timeout = requireScalarValue(fb_dc->ready_timeout(), "DeploymentConfig::ready_timeout");
auto ready_timeout = requireScalarValue(fb_dc->ready_timeout_ms(), "DeploymentConfig::ready_timeout_ms");
if (!ready_timeout.has_value())
{
return score::cpp::make_unexpected(ready_timeout.error());
}
auto shutdown_timeout = requireScalarValue(fb_dc->shutdown_timeout(), "DeploymentConfig::shutdown_timeout");
auto shutdown_timeout =
requireScalarValue(fb_dc->shutdown_timeout_ms(), "DeploymentConfig::shutdown_timeout_ms");
if (!shutdown_timeout.has_value())
{
return score::cpp::make_unexpected(shutdown_timeout.error());
}
auto ready_timeout_ms = secondsToMs(*ready_timeout);
if (!ready_timeout_ms.has_value())
{
LM_LOG_ERROR() << "Invalid value for DeploymentConfig::ready_timeout";
return score::cpp::make_unexpected(ready_timeout_ms.error());
}
auto shutdown_timeout_ms = secondsToMs(*shutdown_timeout);
if (!shutdown_timeout_ms.has_value())
{
LM_LOG_ERROR() << "Invalid value for DeploymentConfig::shutdown_timeout";
return score::cpp::make_unexpected(shutdown_timeout_ms.error());
}
result.ready_timeout_ms = *ready_timeout_ms;
result.shutdown_timeout_ms = *shutdown_timeout_ms;
result.ready_timeout_ms = *ready_timeout;
result.shutdown_timeout_ms = *shutdown_timeout;
result.environmental_variables = convertEnvironmentalVariables(fb_dc->environmental_variables());
result.bin_dir = fb_dc->bin_dir()->str();
result.working_dir = fb_dc->working_dir()->str();
Expand Down Expand Up @@ -505,21 +460,16 @@ score::cpp::expected<RunTargetConfig, IConfigLoader::Error> convertRunTarget(con
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(
fb_rt->recovery_action(),
"RunTarget::recovery_action must never be nullptr as it is required in the schema");
auto transition_timeout = requireScalarValue(fb_rt->transition_timeout(), "RunTarget::transition_timeout");
auto transition_timeout =
requireScalarValue(fb_rt->transition_timeout_ms(), "RunTarget::transition_timeout_ms");
if (!transition_timeout.has_value())
{
return score::cpp::make_unexpected(transition_timeout.error());
}
result.name = fb_rt->name()->str();
result.description = safeString(fb_rt->description());
result.depends_on = convertStringVector(fb_rt->depends_on());
auto transition_timeout_ms = secondsToMs(*transition_timeout);
if (!transition_timeout_ms.has_value())
{
LM_LOG_ERROR() << "Invalid value for RunTarget::transition_timeout";
return score::cpp::make_unexpected(transition_timeout_ms.error());
}
result.transition_timeout_ms = *transition_timeout_ms;
result.transition_timeout_ms = *transition_timeout;
result.recovery_action = convertRequiredSwitchRunTargetAction(fb_rt->recovery_action());
}
return result;
Expand All @@ -532,20 +482,14 @@ score::cpp::expected<FallbackRunTargetConfig, IConfigLoader::Error> convertFallb
if (fb_frt != nullptr)
{
auto transition_timeout =
requireScalarValue(fb_frt->transition_timeout(), "FallbackRunTarget::transition_timeout");
requireScalarValue(fb_frt->transition_timeout_ms(), "FallbackRunTarget::transition_timeout_ms");
if (!transition_timeout.has_value())
{
return score::cpp::make_unexpected(transition_timeout.error());
}
result.description = safeString(fb_frt->description());
result.depends_on = convertStringVector(fb_frt->depends_on());
auto transition_timeout_ms = secondsToMs(*transition_timeout);
if (!transition_timeout_ms.has_value())
{
LM_LOG_ERROR() << "Invalid value for FallbackRunTarget::transition_timeout";
return score::cpp::make_unexpected(transition_timeout_ms.error());
}
result.transition_timeout_ms = *transition_timeout_ms;
result.transition_timeout_ms = *transition_timeout;
}
return result;
}
Expand All @@ -557,18 +501,12 @@ score::cpp::expected<AliveSupervisionConfig, IConfigLoader::Error> convertAliveS
{
return AliveSupervisionConfig{};
}
auto evaluation_cycle = requireScalarValue(fb_as->evaluation_cycle(), "AliveSupervision::evaluation_cycle");
auto evaluation_cycle = requireScalarValue(fb_as->evaluation_cycle_ms(), "AliveSupervision::evaluation_cycle_ms");
if (!evaluation_cycle.has_value())
{
return score::cpp::make_unexpected(evaluation_cycle.error());
}
auto evaluation_cycle_ms = secondsToMs(*evaluation_cycle);
if (!evaluation_cycle_ms.has_value())
{
LM_LOG_ERROR() << "Invalid value for AliveSupervision::evaluation_cycle";
return score::cpp::make_unexpected(evaluation_cycle_ms.error());
}
return AliveSupervisionConfig{*evaluation_cycle_ms};
return AliveSupervisionConfig{*evaluation_cycle};
}

score::cpp::expected<std::optional<WatchdogConfig>, IConfigLoader::Error> convertWatchdog(const fb::Watchdog* fb_wd)
Expand All @@ -579,7 +517,7 @@ score::cpp::expected<std::optional<WatchdogConfig>, IConfigLoader::Error> conver
}
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(
fb_wd->device_file_path(), "Watchdog::device_file_path must never be nullptr as it is required in the schema");
auto max_timeout = requireScalarValue(fb_wd->max_timeout(), "Watchdog::max_timeout");
auto max_timeout = requireScalarValue(fb_wd->max_timeout_ms(), "Watchdog::max_timeout_ms");
if (!max_timeout.has_value())
{
return score::cpp::make_unexpected(max_timeout.error());
Expand All @@ -597,13 +535,7 @@ score::cpp::expected<std::optional<WatchdogConfig>, IConfigLoader::Error> conver
}
WatchdogConfig result{};
result.device_file_path = fb_wd->device_file_path()->str();
auto max_timeout_ms = secondsToMs(*max_timeout);
if (!max_timeout_ms.has_value())
{
LM_LOG_ERROR() << "Invalid value for Watchdog::max_timeout";
return score::cpp::make_unexpected(max_timeout_ms.error());
}
result.max_timeout_ms = *max_timeout_ms;
result.max_timeout_ms = *max_timeout;
result.deactivate_on_shutdown = *deactivate_on_shutdown;
result.require_magic_close = *require_magic_close;
return std::optional<WatchdogConfig>{std::move(result)};
Expand Down
23 changes: 9 additions & 14 deletions score/launch_manager/src/daemon/src/configuration/details/flatbuffer_type_converters.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,16 @@ namespace details
template <typename TargetT>
score::cpp::expected<TargetT, IConfigLoader::Error> validateRange(int64_t value, const std::string_view field_name)
{
// Asserts ensure that std::numeric_limits<TargetT>::min() and std::numeric_limits<TargetT>::max() can be
// Asserts ensure that std::numeric_limits<TargetT>::min() and std::numeric_limits<TargetT>::max() can be
// safely cast to int64_t for the range check:
// Case 1: TargetT is unsigned and smaller than int64_t, so max fits in int64_t and min is 0
// Case 2: TargetT is signed and smaller than int64_t, so both min and max fit in int64_t
// Case 3: TargetT is signed and exactly int64_t, so min and max are the full int64_t range

static_assert(std::numeric_limits<TargetT>::is_integer, "TargetT must be an integer type");
static_assert(sizeof(TargetT) < sizeof(int64_t) ||
(sizeof(TargetT) == sizeof(int64_t) && std::is_signed_v<TargetT>),
"TargetT max must be representable as int64_t");
static_assert(
sizeof(TargetT) < sizeof(int64_t) || (sizeof(TargetT) == sizeof(int64_t) && std::is_signed_v<TargetT>),
"TargetT max must be representable as int64_t");

if (value < static_cast<int64_t>(std::numeric_limits<TargetT>::min()) ||
value > static_cast<int64_t>(std::numeric_limits<TargetT>::max()))
Expand All @@ -63,15 +63,12 @@ score::cpp::expected<TargetT, IConfigLoader::Error> validateRange(int64_t value,

// --- Scalar and enum helpers ---

/// @brief Converts a time value from seconds to milliseconds.
[[nodiscard]] score::cpp::expected<uint32_t, IConfigLoader::Error> secondsToMs(double seconds);
/// @brief Converts a FlatBuffer ApplicationType enum to the config ApplicationType.
[[nodiscard]] ApplicationType convertApplicationType(fb::ApplicationType fb_type);
/// @brief Converts a FlatBuffer ProcessState enum to the config ProcessState.
[[nodiscard]] ProcessState convertProcessState(fb::ProcessState fb_state);
/// @brief Converts a FlatBuffer SchedulingPolicy enum to a POSIX scheduling policy constant.
[[nodiscard]] score::cpp::expected<int32_t, IConfigLoader::Error> convertSchedulingPolicy(
fb::SchedulingPolicy policy);
[[nodiscard]] score::cpp::expected<int32_t, IConfigLoader::Error> convertSchedulingPolicy(fb::SchedulingPolicy policy);

// --- String and vector helpers ---

Expand All @@ -93,16 +90,15 @@ score::cpp::expected<TargetT, IConfigLoader::Error> validateRange(int64_t value,
[[nodiscard]] score::cpp::expected<std::optional<RestartAction>, IConfigLoader::Error> convertRestartAction(
const fb::RestartAction* ra);
/// @brief Converts a FlatBuffer SwitchRunTargetAction to a config SwitchRunTargetAction, or nullopt if absent.
[[nodiscard]] std::optional<SwitchRunTargetAction> convertSwitchRunTargetAction(
const fb::SwitchRunTargetAction* sa);
[[nodiscard]] std::optional<SwitchRunTargetAction> convertSwitchRunTargetAction(const fb::SwitchRunTargetAction* sa);
/// @brief Converts a required FlatBuffer SwitchRunTargetAction; asserts if null.
[[nodiscard]] SwitchRunTargetAction convertRequiredSwitchRunTargetAction(const fb::SwitchRunTargetAction* sa);

// --- Component converters ---

/// @brief Converts a FlatBuffer ComponentAliveSupervision to the config equivalent.
[[nodiscard]] score::cpp::expected<ComponentAliveSupervision, IConfigLoader::Error>
convertComponentAliveSupervision(const fb::ComponentAliveSupervision* fb_cas);
[[nodiscard]] score::cpp::expected<ComponentAliveSupervision, IConfigLoader::Error> convertComponentAliveSupervision(
const fb::ComponentAliveSupervision* fb_cas);
/// @brief Converts a FlatBuffer ApplicationProfile to the config equivalent.
[[nodiscard]] score::cpp::expected<ApplicationProfile, IConfigLoader::Error> convertApplicationProfile(
const fb::ApplicationProfile* fb_ap);
Expand All @@ -127,8 +123,7 @@ convertComponentAliveSupervision(const fb::ComponentAliveSupervision* fb_cas);
// --- Run target converters ---

/// @brief Converts a single FlatBuffer RunTarget to a RunTargetConfig.
[[nodiscard]] score::cpp::expected<RunTargetConfig, IConfigLoader::Error> convertRunTarget(
const fb::RunTarget* fb_rt);
[[nodiscard]] score::cpp::expected<RunTargetConfig, IConfigLoader::Error> convertRunTarget(const fb::RunTarget* fb_rt);
/// @brief Converts a FlatBuffer FallbackRunTarget to a FallbackRunTargetConfig.
[[nodiscard]] score::cpp::expected<FallbackRunTargetConfig, IConfigLoader::Error> convertFallbackRunTarget(
const fb::FallbackRunTarget* fb_frt);
Expand Down
Loading
Loading