Skip to content

Support for loading multiple configurations as addons - #762

Draft
SebSparrowHawk wants to merge 2 commits into
eclipse-score:mainfrom
SebSparrowHawk:ssp_runtime_addons
Draft

Support for loading multiple configurations as addons#762
SebSparrowHawk wants to merge 2 commits into
eclipse-score:mainfrom
SebSparrowHawk:ssp_runtime_addons

Conversation

@SebSparrowHawk

Copy link
Copy Markdown
Contributor

No description provided.

Comment on lines +87 to +101
"global": {
"asil-level": "B",
"applicationID": 1234,
"queue-size": {
"QM-receiver": 8,
"B-receiver": 5,
"B-sender": 12
},
"shm-size-calc-mode": "SIMULATION"
},
"tracing": {
"enable": true,
"applicationInstanceID": "ara_com_example",
"traceFilterConfigPath": "./mw_com_trace_filter.json"
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These are parts of the "AddOn" configuration that we should not allow IMHO. A library provider cannot know global values!

Comment thread score/mw/com/impl/runtime.h Outdated
Comment on lines +165 to +168
/// \brief flag, to identify if an add-on configuration has already been loaded, which has been handled as the
/// initial configuration. Used to identify that such a configuration has been loaded before the user explicitly
/// loaded a configuration.
static bool addon_configuration_loaded_;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would not do that. IMHO we should never load an "AddOn" configuration as initial configuration. This can only lead to problems.

I would clearly state, that AddOn configurations can only be loaded after mw::com was correctly initalized.

Or we make it completly interexchangeable, but then we should not use this variable.

#include <score/assert.hpp>
#include <score/utility.hpp>

#include <exception>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this include required?


std::vector<InstanceIdentifier> Runtime::resolve(const InstanceSpecifier& specifier) const
{
std::lock_guard<std::mutex> lock{configuration_mutex_};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So you added here locks for what reason? To ensure that we have no race condition of using the configuration while extending it?

How would we handle configuration usages from e.g. Proxy / Skeleton creations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

2 participants