Skip to content
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

Jump to bottom

[lgwebos] Update the thing configuration only if the key is different #7302

Merged
merged 2 commits into from
Apr 5, 2020
Merged

[lgwebos] Update the thing configuration only if the key is different #7302

merged 2 commits into from
Apr 5, 2020

Conversation

lolodomo
Copy link
Contributor

@lolodomo lolodomo commented Apr 5, 2020 •
edited
Loading

Signed-off-by: Laurent Garnier [email protected]

@lolodomo
[lgwebos] Update the thing configuration only if the key is different
57f4d57 
Avoid concurrent edit of thing configuration/properties

Signed-off-by: Laurent Garnier <[email protected]>
@lolodomo lolodomo requested a review from sprehn as a code owner April 5, 2020 12:03
@TravisBuddy
Copy link

Travis tests were successful

Hey @lolodomo,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

cpmeister
cpmeister reviewed Apr 5, 2020
View reviewed changes
...nding.lgwebos/src/main/java/org/openhab/binding/lgwebos/internal/handler/LGWebOSHandler.java Outdated
}

@Override
public void storeProperties(Map<String, String> properties) {
public synchronized void storeProperties(Map<String, String> properties) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why synchronized?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not the case with the current code but with another of my changes, you could have different threads calling storeProperties at the same time.
By the way, I see that there is an object synchronizetion inside updateProperties and updateConfiguration so this is very probably useless to add one at the binding level.

@TravisBuddy
Copy link

Travis tests were successful

Hey @lolodomo,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

cpmeister
cpmeister approved these changes Apr 5, 2020
View reviewed changes
Copy link
Contributor

@cpmeister cpmeister left a comment

Choose a reason for hiding this comment

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

LGTM

@cpmeister cpmeister merged commit 46c6cab into openhab:2.5.x Apr 5, 2020
@cpmeister cpmeister added this to the 2.5.4 milestone Apr 5, 2020
@lolodomo lolodomo deleted the lgwebos_update_config branch April 5, 2020 19:22
Hans-Reiner pushed a commit to Hans-Reiner/openhab2-addons that referenced this pull request Apr 11, 2020
@lolodomo @Hans-Reiner
[lgwebos] Update the thing configuration only if the key is different (…
81e6514 
…openhab#7302)

* [lgwebos] Update the thing configuration only if the key is different

Avoid concurrent edit of thing configuration/properties

Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: Hans-Reiner Hoffmann <[email protected]>
yfre pushed a commit to yfre/openhab-addons that referenced this pull request Apr 27, 2020
@lolodomo
[lgwebos] Update the thing configuration only if the key is different (…
213a609 
…openhab#7302)

* [lgwebos] Update the thing configuration only if the key is different

Avoid concurrent edit of thing configuration/properties

Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: Eugen Freiter <[email protected]>
markus7017 pushed a commit to markus7017/openhab-addons that referenced this pull request May 29, 2020
@lolodomo @markus7017
[lgwebos] Update the thing configuration only if the key is different (…
64fbb56 
…openhab#7302)

* [lgwebos] Update the thing configuration only if the key is different

Avoid concurrent edit of thing configuration/properties

Signed-off-by: Laurent Garnier <[email protected]>
LoungeFlyZ pushed a commit to LoungeFlyZ/openhab2-addons that referenced this pull request Jun 8, 2020
@lolodomo @LoungeFlyZ
[lgwebos] Update the thing configuration only if the key is different (…
881ab8a 
…openhab#7302)

* [lgwebos] Update the thing configuration only if the key is different

Avoid concurrent edit of thing configuration/properties

Signed-off-by: Laurent Garnier <[email protected]>
J-N-K pushed a commit to J-N-K/openhab-addons that referenced this pull request Jul 14, 2020
@lolodomo @J-N-K
[lgwebos] Update the thing configuration only if the key is different (…
e62de1a 
…openhab#7302)

* [lgwebos] Update the thing configuration only if the key is different

Avoid concurrent edit of thing configuration/properties

Signed-off-by: Laurent Garnier <[email protected]>
CSchlipp pushed a commit to CSchlipp/openhab-addons that referenced this pull request Jul 26, 2020
@lolodomo @CSchlipp
[lgwebos] Update the thing configuration only if the key is different (…
2e030bb 
…openhab#7302)

* [lgwebos] Update the thing configuration only if the key is different

Avoid concurrent edit of thing configuration/properties

Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: CSchlipp <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
@lolodomo @andrewfg
[lgwebos] Update the thing configuration only if the key is different (…
1113c31 
…openhab#7302)

* [lgwebos] Update the thing configuration only if the key is different

Avoid concurrent edit of thing configuration/properties

Signed-off-by: Laurent Garnier <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
@lolodomo @andrewfg
[lgwebos] Update the thing configuration only if the key is different (…
0d3ffd1 
…openhab#7302)

* [lgwebos] Update the thing configuration only if the key is different

Avoid concurrent edit of thing configuration/properties

Signed-off-by: Laurent Garnier <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
@lolodomo @andrewfg
[lgwebos] Update the thing configuration only if the key is different (…
92060d1 
…openhab#7302)

* [lgwebos] Update the thing configuration only if the key is different

Avoid concurrent edit of thing configuration/properties

Signed-off-by: Laurent Garnier <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
@lolodomo @andrewfg
[lgwebos] Update the thing configuration only if the key is different (…
45df818 
…openhab#7302)

* [lgwebos] Update the thing configuration only if the key is different

Avoid concurrent edit of thing configuration/properties

Signed-off-by: Laurent Garnier <[email protected]>
DaanMeijer pushed a commit to DaanMeijer/openhab-addons that referenced this pull request Sep 1, 2020
@lolodomo @DaanMeijer
[lgwebos] Update the thing configuration only if the key is different (…
dd54f34 
…openhab#7302)

* [lgwebos] Update the thing configuration only if the key is different

Avoid concurrent edit of thing configuration/properties

Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: Daan Meijer <[email protected]>
markus7017 pushed a commit to markus7017/openhab-addons that referenced this pull request Sep 19, 2020
@lolodomo @markus7017
[lgwebos] Update the thing configuration only if the key is different (…
c0a4b82 
…openhab#7302)

* [lgwebos] Update the thing configuration only if the key is different

Avoid concurrent edit of thing configuration/properties

Signed-off-by: Laurent Garnier <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cpmeister cpmeister cpmeister approved these changes

@sprehn sprehn Awaiting requested review from sprehn sprehn is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.5.4
Development

Successfully merging this pull request may close these issues.
3 participants
@lolodomo @TravisBuddy @cpmeister