homebridge / plugins

Information and resources for Homebridge plugins.
https://homebridge.io/w/Verified-Plugins
GNU General Public License v3.0
368 stars 17 forks source link

homebridge-dreo #538

Closed zyonse closed 1 year ago

zyonse commented 1 year ago

Link To GitHub Repo

https://github.com/zyonse/homebridge-dreo

Link To NPM Package

https://www.npmjs.com/package/homebridge-dreo

github-actions[bot] commented 1 year ago

:white_check_mark: Pre-checks completed successfully.

bwp91 commented 1 year ago

Hi @zyonse

Installing the plugin and immediately clicking save in the homebridge ui resulted in this config:

        {
            "options": {},
            "name": "Dreo Platform",
            "platform": "DreoPlatform"
        }

and I had to disable the plugin since it sent my homebridhe into a restart loop:

[25/04/2023, 04:27:11] Homebridge v2.0.0-beta.1 (HAP v1.0.0-beta.8) (Homebridge) is running on port 51781.
[25/04/2023, 04:27:12] TypeError: Cannot read properties of undefined (reading 'access_token')
    at DreoPlatform.discoverDevices (/usr/local/lib/node_modules/homebridge-dreo/src/platform.ts:61:61)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)

Please ensure that all errors are caught properly by the plugin so it does not affect the running of other plugins!

Please also check things like the following just to make sure your plugin handles them correctly:

Of course it's totally okay that your plugin logs an error in these cases - so long as the user can look at the logs and understand what has gone wrong - and most importantly your plugin can stop working without affecting other plugins that a user may have installed.

zyonse commented 1 year ago

@bwp91 Thanks for the feedback and suggestions. I added some actual error handling with console messages to address those login issues, they're included in v1.2.0 of the plugin.

zyonse commented 1 year ago

/check

github-actions[bot] commented 1 year ago

:white_check_mark: Pre-checks completed successfully.

bwp91 commented 1 year ago

Great, for the purposes of verification, the change you have made is perfect.

However, just wanted to query a real-case usage situation, as I noticed that if user/pass is incorrect you remove all the devices:


    if (email === undefined || password === undefined) {
      this.api.unregisterPlatformAccessories(
        PLUGIN_NAME,
        PLATFORM_NAME,
        this.accessories,
      );
      this.log.error('error: Invalid email and/or password');
      return;
    }

Let's say a user has 10 devices with a dreo account and then changes their password in the app. They then restart homebridge without remembering to update their password in homebridge and suddenly all their devices have been removed! would it not be better to just return; without removing devices? just a thought 😀

Will also post a checklist now as there is one other thing I have noticed with regards to github releases

github-actions[bot] commented 1 year ago

Please remember to create a github release with each version of the plugin you release - as this is the information that is shown in the homebridge ui when a user updates the plugin

https://github.com/zyonse/homebridge-dreo/releases

bwp91 commented 1 year ago

...also that for example an npm release of 1.0.0 should have a github tag of v1.0.0

zyonse commented 1 year ago

Let's say a user has 10 devices with a dreo account and then changes their password in the app. They then restart homebridge without remembering to update their password in homebridge and suddenly all their devices have been removed! would it not be better to just return; without removing devices? just a thought 😀

The code you referenced would only be triggered if the user removed their username or password from the config file. If they changed their account password in the Dreo app without updating it in homebridge, the plugin would receive an invalid access_token and just 'return;'

https://github.com/zyonse/homebridge-dreo/blob/1e3118c958c6c3ec207fc2a27d6bca47d43bfd53/src/platform.ts#L71-L76

Also, just added a github release for the latest npm version.

bwp91 commented 1 year ago

Apologies yes you are correct - I misread the code! All with good intentions! Fab - please continue with the github releases and i will verify the plugin now 😁

github-actions[bot] commented 1 year ago

Congratulations! Your plugin has been verified.

You can now add the Verified by Homebridge badge to your plugin's README:

verified-by-homebridge

[![verified-by-homebridge](https://badgen.net/badge/homebridge/verified/purple)](https://github.com/homebridge/homebridge/wiki/Verified-Plugins)

Your plugin is now also eligible to display a :heart: Donate button on its tile in the Homebridge UI. See https://github.com/oznu/homebridge-config-ui-x/wiki/Developers:-Donation-Links for instructions.

If for any reason in the future you can no longer maintain your plugin, please consider transferring it to our unmaintained plugins repo. We can take ownership until another willing developer comes along.

Thank you for your contribution to the Homebridge Community. https://homebridge.io

zyonse commented 1 year ago

Will do, thank you!