Skip to content
This repository was archived by the owner on Aug 5, 2022. It is now read-only.

Conversation

@tcahuzax
Copy link
Contributor

@tcahuzax tcahuzax commented Nov 30, 2015

Signed-off-by: Thomas Cahuzac [email protected]

Don't need to do it though.


<a href='https://www.codereviewhub.com/01org/parameter-framework/pull/317?mark_as_completed=1'><img src='http://www.codereviewhub.com/site/github-mark-as-completed.png' height=26></a>&nbsp;<a href='https://www.codereviewhub.com/01org/parameter-framework/pull/317?approve=1'><img src='http://www.codereviewhub.com/site/github-approve.png' height=26></a>&nbsp;<a href='https://github.com/01org/parameter-framework/pull/317'><img src='http://www.codereviewhub.com/site/github-refresh.png' height=26></a>
<a href='#crh-end'></a>
```
***

@tcahuzax tcahuzax changed the title Auto-sync can be enabled/disabled regardless tuning mode Decorrelating auto-sync and tuning-mode Dec 2, 2015
@tcahuzax tcahuzax force-pushed the autosync_update branch 3 times, most recently from 26b00cb to 0715449 Compare December 2, 2015 11:28
@tcahuzax tcahuzax changed the title Decorrelating auto-sync and tuning-mode [review] Decorrelating auto-sync and tuning-mode Dec 2, 2015
@tcahuzax
Copy link
Contributor Author

tcahuzax commented Dec 2, 2015

@dawagner @krocard plz review

Currently auto-sync can be disabled in
tuning mode only.

This patch decorrelates auto-sync and
tnuning mode features: they can be
activated independently.

Signed-off-by: Thomas Cahuzac <[email protected]>
@tcahuzax tcahuzax force-pushed the autosync_update branch 2 times, most recently from dcc8877 to 6a269f8 Compare December 3, 2015 12:58
@dawagner
Copy link
Contributor

dawagner commented Dec 3, 2015

In the "Adding the ability to set configuration value" and "Allowing to choose the subsystem type" commit messages, please add a context information in the headline: as they stand, they seem to suggest, for instance, that the Parameter Framework does not provide the ability to set a configuration value at all.

@tcahuzax tcahuzax force-pushed the autosync_update branch 4 times, most recently from 909b8e5 to caa584c Compare December 3, 2015 15:50
@tcahuzax
Copy link
Contributor Author

tcahuzax commented Dec 3, 2015

@dawagner @krocard plz review

Copy link
Contributor

Choose a reason for hiding this comment

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

(const std::string& name, core::log::Logger& logger)

Copy link
Contributor

Choose a reason for hiding this comment

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

hungarian

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove virtual as it is redundant with override.

Copy link
Contributor

Choose a reason for hiding this comment

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

ALWAYS_ASSERT(geParameterType(instanceConfigurableElement)->isScalar(),
              "Parameter shall be scalar");

Don't need to do it though.

Functional testing auto-sync feature requires
a fake subsystem that allows to retrieve parameter
value set by the parameter-framework.

This patch introduces the introspection-subsystem,
it provides the ability to retrieve the value
of a boolean parameter.

Signed-off-by: Thomas Cahuzac <[email protected]>
This patch allows the pfw test wrapper to set configuration values.

Signed-off-by: Thomas Cahuzac <[email protected]>
Currently the subsystem type used by the
functional test framework is always "Virtual".

This patch provides the ability to change it.

Signed-off-by: Thomas Cahuzac <[email protected]>
This feature was not test.

This patch introduces a functional test
that uses the instrospection subsystem
to check the auto-sync feature.

Two sequences are tested:
- when auto-sync is activated, parameter sync
  occurs when a parameter is changed
  in the blackboard.
- when auto-sync is off, parameter sync
  occurs when tha auto-sync is turned to on.

Signed-off-by: Thomas Cahuzac <[email protected]>
@tcahuzax
Copy link
Contributor Author

tcahuzax commented Dec 3, 2015

👍
@dawagner @krocard plz review

@krocard
Copy link
Contributor

krocard commented Dec 3, 2015

You still have 1 alignment wrong: #317 (comment)
👍 after that.

dawagner added a commit that referenced this pull request Dec 4, 2015
Decorrelating auto-sync and tuning mode

Auto-sync and tuning mode can now be activated independently.
@dawagner dawagner merged commit 360a6be into intel:master Dec 4, 2015
@krocard krocard changed the title [review] Decorrelating auto-sync and tuning-mode Decorrelating auto-sync and tuning-mode Dec 29, 2015
@coveralls
Copy link

Coverage Status

Coverage decreased (-43.3%) to 31.67% when pulling 6aa22a1 on tcahuzax:autosync_update into 705008b on 01org:master.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants