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

Verification Request: homebridge-sleepme #757

Open
LukeWinikates opened this issue Sep 7, 2024 · 5 comments
Open

Verification Request: homebridge-sleepme #757

LukeWinikates opened this issue Sep 7, 2024 · 5 comments
Labels
awaiting-changes use after review has started - awaiting user to make changes to plugin request-verification

Comments

@LukeWinikates
Copy link

Plugin Name

homebridge-sleepme

Link To GitHub Repo

https://github.com/LukeWinikates/homebridge-sleepme

Plugin Icon (Optional)

No response

The plugin does not offer the same nor less functionality than that of any existing verified plugin.

🟢 Yes

The plugin successfully installs and does not start unless it is configured.

🟢 Yes

The plugin does not require the user to run Homebridge in a TTY or with non-standard startup parameters, even for initial configuration.

🟢 Yes

The plugin does not contain any analytics or calls that enable you to track the user.

🟢 Yes

If the plugin needs to write files to disk (cache, keys, etc.), it stores them inside the Homebridge storage directory.

🟢 Yes

The plugin does not throw unhandled exceptions, the plugin must catch and log its own errors.

🟢 Yes

More Information

This is my first Homebridge plugin, and is quite simple. I'd love to receive the feedback that comes with the verification process and improve the plugin accordingly. It's for a niche family of devices, and I don't expect the utilization to ever be very high.

I answered the "The plugin successfully installs and does not start unless it is configured" question as "yes", but I'm not sure I understand what "start" means in this case. I'd be happy to add logic to short-circuit startup if no API keys are configured. As is, if unconfigured, the plugin would find no API keys at startup and wouldn't find anything to start up and loop over.

The device this plugin supports has a water tank, which gets used up over time and needs to be refilled. I wanted to expose this information in the plugin, but the only accessory/characteristic that mapped at all to this was a "battery". I'd love any suggestions for a better way of handling that, if possible.

@LukeWinikates LukeWinikates added the pending the label given to a new verification/icon request label Sep 7, 2024
@bwp91 bwp91 added currently-reviewing use for starting a review - adds a comment with the verification list with empty checkboxes and removed pending the label given to a new verification/icon request labels Sep 22, 2024
Copy link

github-actions bot commented Sep 22, 2024

  • General
    • The plugin must be of type dynamic platform.
    • The plugin must not offer the same nor less functionality than that of any existing verified plugin.
  • Repo
    • The plugin must be published to NPM and the source code available on a GitHub repository, with issues enabled.
    • A GitHub release should be created for every new version of your plugin, with release notes.
  • Environment
    • The plugin must run on all supported LTS versions of Node.js, at the time of writing this is Node v18 and v20.
    • The plugin must successfully install and not start unless it is configured.
    • The plugin must not execute post-install scripts that modify the users' system in any way.
    • The plugin must not require the user to run Homebridge in a TTY or with non-standard startup parameters, even for initial configuration.
  • Codebase
    • The plugin must implement the Homebridge Plugin Settings GUI.
    • The plugin must not contain any analytics or calls that enable you to track the user.
    • If the plugin needs to write files to disk (cache, keys, etc.), it must store them inside the Homebridge storage directory.
    • The plugin must not throw unhandled exceptions, the plugin must catch and log its own errors.

@bwp91
Copy link
Contributor

bwp91 commented Sep 22, 2024

Hi @LukeWinikates

I have installed the plugin and set up as a child bridge. I have the following (minimal) config:

        {
            "platform": "SleepmeHomebridgePlugin",
            "_bridge": {
                "username": "0E:43:1A:1A:1E:EE",
                "port": 37163
            }
        }

This is leading to a crash loop with the following message:

[22/09/2024, 09:41:18] [homebridge-sleepme] Executed didFinishLaunching callback
TypeError: Cannot read properties of undefined (reading 'forEach')
    at SleepmePlatform.discoverDevices (file:///usr/local/lib/node_modules/homebridge-sleepme/src/platform.ts:61:26)
    at HomebridgeAPI.<anonymous> (file:///usr/local/lib/node_modules/homebridge-sleepme/src/platform.ts:40:12)
    at HomebridgeAPI.emit (node:events:519:28)
    at HomebridgeAPI.signalFinished (file:///usr/local/lib/node_modules/homebridge/src/api.ts:244:10)
    at ChildBridgeFork.startBridge (file:///usr/local/lib/node_modules/homebridge/src/childBridgeFork.ts:193:14)
[22/09/2024, 09:41:18] [homebridge-sleepme] Child bridge ended (code 1, signal null). The child bridge ended unexpectedly, which is normally due to the plugin not catching its errors properly. Please report this to the plugin developer by clicking on the 'Report An Issue' option in the plugin menu dropdown from the Homebridge UI. If there are related logs shown above, please include them in your report.

Here you need to check that config.api_keys exists, and is an array!

https://github.com/LukeWinikates/homebridge-sleepme/blob/8aa271badfea4d07d20c6fe63dd6c8b24189f725/src/platform.ts#L61

@bwp91 bwp91 added awaiting-changes use after review has started - awaiting user to make changes to plugin and removed currently-reviewing use for starting a review - adds a comment with the verification list with empty checkboxes labels Sep 22, 2024
@LukeWinikates
Copy link
Author

Thanks @bwp91 - I'll get this fixed soon.

@LukeWinikates
Copy link
Author

@bwp91 in principle, would it be possible for homebridge itself to validate the config against the definition in the schema file used to configure the UI? Is there a way to do that now? I would l like to avoid adding a json schema library to my plugin directly, to minimize dependencies.

I can see plugins using a helper function like matchesPluginConfigSchema(config), and proceeding normally if it's the happy path, or logging an error if it doesn't match. If the configuration schema changes, authors could write their own custom logic to migrate old versions of their plugin's config to the new style.

@LukeWinikates
Copy link
Author

Hi @bwp91 - I just published a new release. This should be ready for you to retest the config behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-changes use after review has started - awaiting user to make changes to plugin request-verification
Projects
None yet
Development

No branches or pull requests

2 participants