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.

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

[homekit] bugfix #7701. set correct configuration revision on bundle start. #7702

Merged
merged 1 commit into from
May 21, 2020

Conversation

yfre
Copy link
Contributor

@yfre yfre commented May 20, 2020

HomeKit uses configuration revision number to indicate changes in the configuration of accessories and bridges.
Bridge (in our case openHAB) communicates the revision number to all clients (home apps) and should increase it with every change in configuration (in our case items config). This is supported by HomeKit addon. However, on HomeKit bundle restart the revision was set to 1.

e.g. before bundle restart the revision was 34. after bundle restart the revision was set to 1.
This could lead in some cases to a configuration reset in home app so that all changes done in home app (e.g. room assignment, scenes, renaming) will get lost. See #7701

This PR ensures that the configuration revision is set correctly on the bundle start.

Signed-off-by: Eugen Freiter [email protected]

@TravisBuddy
Copy link

Travis tests were successful

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

@yfre yfre changed the title [homekit] set correct configuration revision on bundle start. potential fix for… [homekit] bugfix #7701. set correct configuration revision on bundle start. May 20, 2020
@J-N-K
Copy link
Member

J-N-K commented May 20, 2020

LGTM. I was wondering if we should also store the number of detected accessories and only start the bundle after we either found the same number or a certain timespan has elapsed.

The idea behind this was that there may be some delay with the items on startup.

sequence 1:

  1. openHAB starts
  2. items are read
  3. homekit addon starts
  4. app connects and gets all items

sequence 2:

  1. openhab starts
  2. homekit addon starts
  3. app connects and gets no items
  4. items are read and reported to the app

My idea was that in sequence 2 the app removes all accessories (and their metadata) and later re-adds them, but the metadata is lost. Sometimes only some of my items are moved to the standard room and I thought that this might be related to the sequential reading of the items. Also sometimes rules report missing items on startup.

@yfre
Copy link
Contributor Author

yfre commented May 20, 2020

LGTM. I was wondering if we should also store the number of detected accessories and only start the bundle after we either found the same number or a certain timespan has elapsed.

The idea behind this was that there may be some delay with the items on startup.

sequence 1:

  1. openHAB starts
  2. items are read
  3. homekit addon starts
  4. app connects and gets all items

sequence 2:

  1. openhab starts
  2. homekit addon starts
  3. app connects and gets no items
  4. items are read and reported to the app

My idea was that in sequence 2 the app removes all accessories (and their metadata) and later re-adds them, but the metadata is lost. Sometimes only some of my items are moved to the standard room and I thought that this might be related to the sequential reading of the items. Also sometimes rules report missing items on startup.

this is good idea.
is there a way to check with openHAB whether all bundles are loaded and all configs are parsed? e.g. kind of event - openhab started completely?

@J-N-K
Copy link
Member

J-N-K commented May 20, 2020

Unfortunately, since eclipse-archived/smarthome#1896 ist still not resolved, there is no such event.

That‘s probably why the idea of the configurable delay (default 30s) was born in the issue you linked.

@J-N-K
Copy link
Member

J-N-K commented May 21, 2020

Since this a fix anyway, I'll merge that and we can add more improvements later.

@J-N-K J-N-K added the bug An unexpected problem or unintended behavior of an add-on label May 21, 2020
@J-N-K J-N-K added this to the 2.5.6 milestone May 21, 2020
@J-N-K J-N-K linked an issue May 21, 2020 that may be closed by this pull request
@J-N-K J-N-K merged commit c3b58fc into openhab:2.5.x May 21, 2020
@yfre yfre deleted the fix_7701 branch May 21, 2020 22:08
LoungeFlyZ pushed a commit to LoungeFlyZ/openhab2-addons that referenced this pull request Jun 8, 2020
J-N-K pushed a commit to J-N-K/openhab-addons that referenced this pull request Jul 14, 2020
CSchlipp pushed a commit to CSchlipp/openhab-addons that referenced this pull request Jul 26, 2020
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
DaanMeijer pushed a commit to DaanMeijer/openhab-addons that referenced this pull request Sep 1, 2020
markus7017 pushed a commit to markus7017/openhab-addons that referenced this pull request Sep 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of an add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[homekit] home app lose the room assignment
3 participants