Skip to content

ConfigParser Discussion #164

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
Thomas-Mikhael wants to merge 2 commits into
base: main
Choose a base branch
from
Open

ConfigParser Discussion #164

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
d621590
Initial Discussion points
Thomas-Mikhael Feb 20, 2026
4c3b377
Refining
Thomas-Mikhael Feb 20, 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
21 changes: 13 additions & 8 deletions score/mw/com/impl/configuration/config_parser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ namespace

using std::string_view_literals::operator""sv;

// TM: Repeated key definitions in many other files
// binding_service_type_deployment_impl.h
// config_parser.cpp
// lola_service_instance_deployment.cpp
// someip_service_instance_deployment.cpp
// tracing_filter_config_parser.cpp
constexpr auto kServiceInstancesKey = "serviceInstances"sv;
constexpr auto kInstanceSpecifierKey = "instanceSpecifier"sv;
constexpr auto kServiceTypeNameKey = "serviceTypeName"sv;
Expand Down Expand Up @@ -73,10 +79,10 @@ constexpr auto kEventMaxSubscribersKey = "maxSubscribers"sv;
constexpr auto kEventEnforceMaxSamplesKey = "enforceMaxSamples"sv;
constexpr auto kEventMaxConcurrentAllocationsKey = "maxConcurrentAllocations"sv;
constexpr auto kMaxConcurrentAllocationsDefault = static_cast<std::uint8_t>(1U);
constexpr auto kFieldNumberOfSampleSlotsKey = "numberOfSampleSlots"sv;
constexpr auto kFieldNumberOfSampleSlotsKey = "numberOfSampleSlots"sv; //TM: duplicate of kEventNumberOfSampleSlotsKey, Line 71
constexpr auto kFieldMaxSubscribersKey = "maxSubscribers"sv;
constexpr auto kFieldEnforceMaxSamplesKey = "enforceMaxSamples"sv;
constexpr auto kFieldMaxConcurrentAllocationsKey = "maxConcurrentAllocations"sv;
constexpr auto kFieldMaxConcurrentAllocationsKey = "maxConcurrentAllocations"sv; //TM: duplicate of kEventMaxConcurrentAllocationsKey, Line 75
constexpr auto kLolaShmSizeKey = "shm-size"sv;
constexpr auto kLolaControlAsilBShmSizeKey = "control-asil-b-shm-size"sv;
constexpr auto kLolaControlQmShmSizeKey = "control-qm-shm-size"sv;
Expand Down Expand Up @@ -456,7 +462,7 @@ auto ParseLolaFieldInstanceDeployment(const score::json::Object& json_map, LolaS

const auto number_of_sample_slots =
deployment_parser.RetrieveJsonElement<LolaFieldInstanceDeployment::SampleSlotCountType>(
kFieldNumberOfSampleSlotsKey);
kFieldNumberOfSampleSlotsKey); // different handling than Events which xor-ing with numberOfSampleSlots (using GetNumberOfSampleSlots()) Line 405
const auto max_subscribers =
deployment_parser.RetrieveJsonElement<LolaFieldInstanceDeployment::SubscriberCountType>(
kFieldMaxSubscribersKey);
Expand Down Expand Up @@ -488,7 +494,6 @@ auto ParseLolaMethodInstanceDeployment(const score::json::Object& json_map, Lola
{
return;
}

const auto methods_list_result = methods->second.As<score::json::List>();
SCORE_LANGUAGE_FUTURECPP_PRECONDITION_PRD_MESSAGE(methods_list_result.has_value(), "Configuration corrupted, check with json schema");
const auto& methods_list = methods_list_result.value().get();
Expand Down Expand Up @@ -669,7 +674,7 @@ auto ParseServiceInstanceDeployments(const score::json::Object& json_map,
"Configuration corrupted, check with json schema");
const auto& bindingValue = bindingValue_result.value().get();
if (bindingValue == kSomeIpBinding)
{
{ // TM: Here it logs fatal and aborts, while in 974 ParseServiceTypeDeployment() it just skips it
Copy link

@JochenSatETAS JochenSatETAS Feb 23, 2026

Choose a reason for hiding this comment

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

Maybe a more generic question: why do we need the binding twice? Once in the serviceType and once in the serviceInstance.

score::mw::log::LogFatal("lola") << "Provided SOME/IP binding, which can not be parsed.";
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(false);
}
Expand Down Expand Up @@ -931,7 +936,7 @@ auto ParseLoLaServiceTypeDeployments(const score::json::Object& json_map) -> Lol
score::mw::log::LogFatal("lola") << "Configuration should contain at least one event, field, or method.";
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(false);
}
if (!AreEventFieldAndMethodIdsUnique(lola))
if (!AreEventFieldAndMethodIdsUnique(lola))
{
score::mw::log::LogFatal("lola") << "Configuration cannot contain duplicate eventId, fieldId, or methodId.";
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(false);
Expand Down Expand Up @@ -1064,7 +1069,7 @@ auto ParseSenderQueueSize(const score::json::Object& global_config_map) -> score
auto queue_size_obj = queue_size->second.As<json::Object>();
SCORE_LANGUAGE_FUTURECPP_PRECONDITION_PRD_MESSAGE(queue_size_obj.has_value(), "Configuration corrupted, check with json schema");
const auto& queue_size_map = queue_size_obj.value().get();
const auto& asil_tx_queue_size = queue_size_map.find("B-sender");
const auto& asil_tx_queue_size = queue_size_map.find("B-sender"); //TM: No QM-sender
if (asil_tx_queue_size != queue_size_map.cend())
{
return score::ResultToAmpOptionalOrElse(asil_tx_queue_size->second.As<std::int32_t>(), [](const auto&) {
Expand Down Expand Up @@ -1296,7 +1301,7 @@ void CrosscheckServiceInstancesToTypes(const Configuration& config)
const auto& serviceTypeDeployment =
std::get<LolaServiceTypeDeployment>(foundServiceType->second.binding_info_);
const auto search = serviceTypeDeployment.events_.find(eventInstanceElement.first);
if (search == serviceTypeDeployment.events_.cend())
if (search == serviceTypeDeployment.events_.cend()) // TM: there was no check for name duplication during parsing, only ids
{
::score::mw::log::LogFatal("lola")
<< "Service instance " << service_instance.first << "event" << eventInstanceElement.first
Expand Down
3 changes: 3 additions & 0 deletions score/mw/com/impl/configuration/lola_event_instance_deployment.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ constexpr LolaEventInstanceDeployment::TracingSlotSizeType kNumberOfIpcTracingSl

} // namespace

// TM: Constructor is not checking the values (were not also checked in config_parser.cpp, e.g. number_of_sample_slots_)
LolaEventInstanceDeployment::LolaEventInstanceDeployment(std::optional<SampleSlotCountType> number_of_sample_slots,
std::optional<SubscriberCountType> max_subscribers,
std::optional<std::uint8_t> max_concurrent_allocations,
Expand Down Expand Up @@ -146,6 +147,8 @@ auto LolaEventInstanceDeployment::GetNumberOfTracingSlots() const noexcept -> Tr
return number_of_tracing_slots_;
}

// TM: SetNumberOfSampleSlots() is not validating the passed value ( 0 < value <= maxSamples).
// Is this intended that it overwrites the json config value? if yes, at least a check against 0 value is needed
void LolaEventInstanceDeployment::SetNumberOfSampleSlots(SampleSlotCountType number_of_sample_slots) noexcept
{

Expand Down
8 changes: 5 additions & 3 deletions score/mw/com/impl/instance_identifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ InstanceIdentifier::InstanceIdentifier(const json::Object& json_object, std::str
if (serialization_version != serializationVersion)
{
::score::mw::log::LogFatal("lola") << "InstanceIdentifier serialization versions don't match. "
<< serialization_version << "!=" << serializationVersion << ". Terminating.";
<< serialization_version << "!=" << serializationVersion << ". Terminating.";
std::terminate();
}

Expand All @@ -96,17 +96,18 @@ InstanceIdentifier::InstanceIdentifier(const json::Object& json_object, std::str
ServiceTypeDeployment type_deployment{GetValueFromJson<json::Object>(json_object, kServiceTypeDeploymentKey)};

auto service_identifier_type = instance_deployment.service_;
// TM: is this a DeadCode Constructor? What is its use?
const auto* const type_deployment_ptr =
configuration_->AddServiceTypeDeployment(std::move(service_identifier_type), std::move(type_deployment));
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(type_deployment_ptr != nullptr,
"Could not insert service type deployment into configuration map");
"Could not insert service type deployment into configuration map");
type_deployment_ = type_deployment_ptr;

auto instance_specifier = instance_deployment.instance_specifier_;
const auto* const service_instance_deployment_ptr =
configuration_->AddServiceInstanceDeployments(std::move(instance_specifier), std::move(instance_deployment));
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(service_instance_deployment_ptr != nullptr,
"Could not insert instance deployment into configuration map");
"Could not insert instance deployment into configuration map");
instance_deployment_ = service_instance_deployment_ptr;
}

Expand All @@ -123,6 +124,7 @@ InstanceIdentifier::InstanceIdentifier(const ServiceInstanceDeployment& deployme
{
}

// TM: What are the use cases of all Serialize() functions
auto InstanceIdentifier::Serialize() const noexcept -> json::Object
{
json::Object json_object{};
Expand Down
14 changes: 9 additions & 5 deletions score/mw/com/impl/runtime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ namespace
inline void warn_double_init()
{
score::mw::log::LogWarn("lola") << "score::mw::com::impl::Runtime is already initialized! Redundant call to a "
"Runtime::Initialize() overload within production code needs to be checked.";
"Runtime::Initialize() overload within production code needs to be checked.";
}

inline void error_double_init()
Expand Down Expand Up @@ -97,7 +97,8 @@ score::cpp::optional<TracingFilterConfig> ParseTraceConfig(const Configuration&
score::Result<TracingFilterConfig> tracing_config_result = tracing::Parse(trace_filter_config_path, configuration);
if (!tracing_config_result.has_value())
{
score::mw::log::LogError("lola") << "Parsing tracing config failed with error: " << tracing_config_result.error();
score::mw::log::LogError("lola") << "Parsing tracing config failed with error: "
<< tracing_config_result.error();
return {};
}
return std::move(tracing_config_result).value();
Expand Down Expand Up @@ -190,10 +191,11 @@ Runtime::Runtime(std::pair<Configuration&&, score::cpp::optional<TracingFilterCo
// LCOV_EXCL_STOP
for (auto& binding_runtime : binding_runtimes_)
{
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(binding_runtime.second->GetTracingRuntime() != nullptr,
"Binding specific runtime has no tracing runtime although tracing is enabled!");
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(
binding_runtime.second->GetTracingRuntime() != nullptr,
"Binding specific runtime has no tracing runtime although tracing is enabled!");
score::cpp::ignore = binding_tracing_runtimes.emplace(binding_runtime.first,
binding_runtime.second->GetTracingRuntime());
binding_runtime.second->GetTracingRuntime());
}
tracing_runtime_ = std::make_unique<tracing::TracingRuntime>(std::move(binding_tracing_runtimes));
}
Expand All @@ -217,6 +219,8 @@ std::vector<InstanceIdentifier> Runtime::resolve(const InstanceSpecifier& specif
// instance is available, there is also a matching service type available. Because parsing of the configuration
// is automatically done before instantiating the runtime, this condition is always positive. To increase the
// robustness of the code, we still check for this condition.

// TM: Will we remove such extra robustness check?
if (type_deployment != configuration_.GetServiceTypes().cend())
{
result.push_back(make_InstanceIdentifier(instanceSearch->second, type_deployment->second));
Expand Down
3 changes: 2 additions & 1 deletion score/mw/com/impl/service_discovery.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,8 @@ auto ServiceDiscovery::GetServiceDiscoveryClient(const InstanceIdentifier& insta
mw::log::LogFatal("lola") << "Service discovery failed to find fitting binding for"
<< instance_identifier.ToString();
}
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(binding_runtime != nullptr, "Unsupported binding");
// TM: This assertion is redundant (Line 298)
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(binding_runtime != nullptr, "Unsupported binding");

return binding_runtime->GetServiceDiscoveryClient();
}
Expand Down