search

lucacalcaterra / org.openhab.binding.melcloud

MelCloud binding for Openhab.
10 stars 6 forks source link

Many improvements #8

Closed paulianttila closed 5 years ago

paulianttila commented 5 years ago

There is still race condition possibility in deviceStatus handling (MelCloudDeviceHandler). deviceStatus is periodically updated by scheduler and if command is send exactly in the same time, deviceStatus might be corrupted as cmdtoSend is just reference to same deviceStatus.

I don't fully understand the MELCloud effectingFlag and hasPendingCommand concept, so for me it's rather difficult to fix this issue.

paulianttila commented 5 years ago

I modified deviceStatus sending to contain only needed fields (hopefully). Previously, binding send the whole deviceStatus including e.g. wether forecast information back to MELCloud.

paulianttila commented 5 years ago

Race condition possibility should be now fixed (not 100% happy about the solution).

paulianttila commented 5 years ago

@lucacalcaterra, ready to merge

lucacalcaterra commented 5 years ago

@lucacalcaterra, ready to merge

I've no time to study the changes.

I trust your work and I merge. ok ?

lucacalcaterra commented 5 years ago

it's right to include in git repo the .gitignore files and .vscode (for build the jar) ? or leave it like that ?

lucacalcaterra commented 5 years ago

or ... fork the official openhab2-addons repo and merge our binding (instead of it's alone ) ?

paulianttila commented 5 years ago

it's right to include in git repo the .gitignore files and .vscode

openhab2-addons repo contains already .gitignore file (see https://github.com/openhab/openhab2-addons/blob/master/.gitignore). As you can see .vscode is also ignored there.

You should make full fork from openhab2-addons repo and merge melcloud binding there. Please, create e.g. melcloud branch and do not merge directly to the master branch. After that we can modify karaf feature files, etc before PR.

You should also sign the work accordingly (see https://www.openhab.org/docs/developer/contributing.html) and also add me by "Also-by: " tag.

lucacalcaterra commented 5 years ago

@paulianttila I created a repo forked from official oh2-addons and started a new branch named melcloud on it, copying the melcloud binding on the bundles dir (as it is , losting all commits history, a new, zero copy)

I invited you, let me know if it's ok and we can start to adjust for submit a PR

paulianttila commented 5 years ago

Thanks Luca, I might have couple of hours today, but if not I will continue during next week.

lucacalcaterra commented 5 years ago

I realized that the repo is probably not good. I ran the setup for VsCode but the Openhab team considered experimental VsCode. I'll probably have to recreate the branch. I'll update you

Il giorno gio 27 giu 2019 alle ore 11:44 pali notifications@github.com ha scritto:

Thanks Luca, I might have couple of hours today, but if not I will continue during next week.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/lucacalcaterra/org.openhab.binding.melcloud/pull/8?email_source=notifications&email_token=AAGWZUK45YB6TTNXWNBMV6DP4SDWFA5CNFSM4H2733SKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYWSMQQ#issuecomment-506275394, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGWZUIZAZGUQQNHFVPLY7LP4SDWFANCNFSM4H2733SA .

lucacalcaterra commented 5 years ago

@paulianttila Do you use vscode or eclipse for development ?

paulianttila commented 5 years ago

I use Eclipse

lucacalcaterra commented 5 years ago

I recreated repo and melcloud branch, invited you as collaborator.

Let me know if steps i've done are corrects.

Luca

Il giorno gio 27 giu 2019 alle ore 15:58 pali notifications@github.com ha scritto:

I use Eclipse

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/lucacalcaterra/org.openhab.binding.melcloud/pull/8?email_source=notifications&email_token=AAGWZUJ5WGXNSIYWMOCZKGDP4TBP7A5CNFSM4H2733SKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYXGPRY#issuecomment-506357703, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGWZUNRPMJXQD5ZAIJVTP3P4TBP7ANCNFSM4H2733SA .