-
Notifications
You must be signed in to change notification settings - Fork 15
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
Js controller7 compatibility #763
Js controller7 compatibility #763
Conversation
for (const objectDevice in objectDevices) { | ||
this.deleteDevice(objectDevice); | ||
} | ||
// const objectDevices: ioBroker.DeviceObject[] = await this.getDevicesAsync(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, commenting this out can result in stale devices not bee cleaned.
We should maybe address this in a follow up issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simply REMOVING deleteDevice is no solution (in my oppinion). You should use the supported methods to delete an object instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this method in the PR, but at 4 a.m., when my IOB restarted, I was woken up by my Ring alerts going off with messages like: ‘There is someone at the door!’ 😂 This happened because all the datapoints were deleted and then rebuilt. So, I quickly went back and commented out that part. I’ll take a closer look later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course deleting ALL states is a bad idea as this would delete all custom settings (i.e. history) too ...
Thanks for checking and sleep well :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This initial "clean up" is a leftover from first adapter implementation from previous repo owner.
Of course a better/cleaner solution would be to e.g. communicate with api and cross reference those devices with the one's in object tree, thus allowing to only delete indeed removed devices.
Or adding an option in adapter settings menu to "Remove old/unreachable devices".
As this PR removes those old lines I did comment about it in the Code-Review, but as previously stated we can address it in a separate issue 👍
526332a
into
iobroker-community-adapters:master
Fixes #707