Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

homebridge-helios-kwl #704

Closed
ubreu opened this issue Mar 19, 2024 · 20 comments
Closed

homebridge-helios-kwl #704

ubreu opened this issue Mar 19, 2024 · 20 comments
Labels
verified use when a plugin meets the criteria - adds the verified badge text

Comments

@ubreu
Copy link

ubreu commented Mar 19, 2024

Plugin Name

@ubreu/homebridge-helios-kwl

Link To GitHub Repo

https://github.com/ubreu/homebridge-helios-kwl

Link To NPM Package

https://www.npmjs.com/package/@ubreu/homebridge-helios-kwl

Plugin Icon (Optional)

No response

@ubreu ubreu added the pending the label given to a new verification/icon request label Mar 19, 2024
Copy link

✅ Pre-checks completed successfully.

@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 Mar 25, 2024
Copy link

github-actions bot commented Mar 25, 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.



Comment /check to run checks again.

@bwp91
Copy link
Contributor

bwp91 commented Mar 25, 2024

Please ensure to validate the configuration.

A config of:

        {
            "platform": "HeliosKwlHomebridgePlugin"
        }

crashes and restarts the process:

[25/03/2024, 19:50:01] Registering platform '@ubreu/homebridge-helios-kwl.HeliosKwlHomebridgePlugin'
[25/03/2024, 19:50:02] [@ubreu/homebridge-helios-kwl] Loaded @ubreu/homebridge-helios-kwl v0.1.0 child bridge successfully

/usr/local/lib/node_modules/@ubreu/homebridge-helios-kwl/node_modules/ws/lib/websocket.js:692
      throw new SyntaxError(`Invalid URL: ${address}`);
            ^
SyntaxError: Invalid URL: ws://undefined:undefined/
    at initAsClient (/usr/local/lib/node_modules/@ubreu/homebridge-helios-kwl/node_modules/ws/lib/websocket.js:692:13)
    at new WebSocket (/usr/local/lib/node_modules/@ubreu/homebridge-helios-kwl/node_modules/ws/lib/websocket.js:85:7)
    at HeliosVentilation.connect (/usr/local/lib/node_modules/@ubreu/homebridge-helios-kwl/src/helios/ventilation.ts:91:16)
    at HeliosVentilation.send (/usr/local/lib/node_modules/@ubreu/homebridge-helios-kwl/src/helios/ventilation.ts:48:27)
    at HeliosVentilationPlatform.addAccessory (/usr/local/lib/node_modules/@ubreu/homebridge-helios-kwl/src/platform.ts:53:32)
    at HomebridgeAPI.<anonymous> (/usr/local/lib/node_modules/@ubreu/homebridge-helios-kwl/src/platform.ts:36:12)
    at HomebridgeAPI.emit (node:events:518:28)
    at HomebridgeAPI.signalFinished (/usr/local/lib/node_modules/homebridge/src/api.ts:275:10)
    at ChildBridgeFork.startBridge (/usr/local/lib/node_modules/homebridge/src/childBridgeFork.ts:189:14)
[25/03/2024, 19:50:02] [@ubreu/homebridge-helios-kwl] Child bridge process ended
[25/03/2024, 19:50:02] [@ubreu/homebridge-helios-kwl] Process Ended. Code: 1, Signal: null

@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 Mar 25, 2024
@ubreu
Copy link
Author

ubreu commented Mar 26, 2024

@bwp91 : Thank you for your feedback. I have fixed this with release 0.1.1

@bwp91
Copy link
Contributor

bwp91 commented Mar 26, 2024

It is important to catch all errors that the plugin may throw. For example with this config:

        {
            "name": "@ubreu/homebridge-helios-kwl",
            "heliosHost": " ",
            "heliosPort": 0,
            "platform": "HeliosKwlHomebridgePlugin"
        }

It is crashing and restarting:

[26/03/2024, 21:26:30] [@ubreu/homebridge-helios-kwl] Loaded @ubreu/homebridge-helios-kwl v0.1.1 child bridge successfully
[26/03/2024, 21:26:30] Loaded 0 cached accessories from cachedAccessories.0E92BA99F44D.

/usr/local/lib/node_modules/@ubreu/homebridge-helios-kwl/node_modules/ws/lib/websocket.js:692
      throw new SyntaxError(`Invalid URL: ${address}`);
            ^
SyntaxError: Invalid URL: ws:// :0/
    at initAsClient (/usr/local/lib/node_modules/@ubreu/homebridge-helios-kwl/node_modules/ws/lib/websocket.js:692:13)
    at new WebSocket (/usr/local/lib/node_modules/@ubreu/homebridge-helios-kwl/node_modules/ws/lib/websocket.js:85:7)
    at HeliosVentilation.connect (/usr/local/lib/node_modules/@ubreu/homebridge-helios-kwl/src/helios/ventilation.ts:91:16)
    at HeliosVentilation.send (/usr/local/lib/node_modules/@ubreu/homebridge-helios-kwl/src/helios/ventilation.ts:48:27)
    at HeliosVentilationPlatform.addAccessory (/usr/local/lib/node_modules/@ubreu/homebridge-helios-kwl/src/platform.ts:58:32)
    at HomebridgeAPI.<anonymous> (/usr/local/lib/node_modules/@ubreu/homebridge-helios-kwl/src/platform.ts:36:12)
    at HomebridgeAPI.emit (node:events:518:28)
    at HomebridgeAPI.signalFinished (/usr/local/lib/node_modules/homebridge/src/api.ts:275:10)
    at ChildBridgeFork.startBridge (/usr/local/lib/node_modules/homebridge/src/childBridgeFork.ts:189:14)
[26/03/2024, 21:26:31] [@ubreu/homebridge-helios-kwl] Child bridge process ended
[26/03/2024, 21:26:31] [@ubreu/homebridge-helios-kwl] Process Ended. Code: 1, Signal: null

@ubreu
Copy link
Author

ubreu commented Mar 27, 2024

@bwp91 : Isn't it sufficient to set the format and the required flag in the config.schema.json?
This prevents setting invalid values in the plugin config UI

Screenshot 2024-03-27 at 08 04 03

@bwp91
Copy link
Contributor

bwp91 commented Mar 27, 2024

Hi @ubreu

It shows these validation warnings but it does not prevent you from clicking on save.

also there are still many users that prefer to configure plugins using json directly.

@bwp91
Copy link
Contributor

bwp91 commented Mar 27, 2024

Here:

https://github.com/ubreu/homebridge-helios-kwl/blob/c92049cf26cad743f00b7930a46213109af690a2/src/platform.ts#L28-L29

I would suggest adding some basic config validation like (feel free to amend as you wish)

    if(!config.heliosHost || typeof config.heliosHost !== 'string' || !config.heliosPort || !Number.isInteger(config.heliosPort) || config.heliosPort < 0) {
      this.log.error('heliosHost and heliosPort have to be configured properly.');
      return;
    }

But also your

https://github.com/ubreu/homebridge-helios-kwl/blob/c92049cf26cad743f00b7930a46213109af690a2/src/platform.ts#L51

function is called asynchronously, so any errors that are thrown inside it will cause the bridge process to crash and restart. For example, even with a valid configuration (host http://localhost port 9090 for example), since the web socket cannot connect to this address on my local machine (since nothing is running on that port), this too leads to a crash and restart.

[27/03/2024, 07:46:24] [@ubreu/homebridge-helios-kwl] Child bridge process ended
[27/03/2024, 07:46:24] [@ubreu/homebridge-helios-kwl] Process Ended. Code: 1, Signal: null
[27/03/2024, 07:46:31] [@ubreu/homebridge-helios-kwl] Restarting Process...
[27/03/2024, 07:46:32] [@ubreu/homebridge-helios-kwl] Launched child bridge with PID 28599
[27/03/2024, 07:46:32] Registering platform '@ubreu/homebridge-helios-kwl.HeliosKwlHomebridgePlugin'
[27/03/2024, 07:46:32] [@ubreu/homebridge-helios-kwl] Loaded @ubreu/homebridge-helios-kwl v0.1.1 child bridge successfully
[27/03/2024, 07:46:32] Loaded 0 cached accessories from cachedAccessories.0E92BA99F44D.
[27/03/2024, 07:46:32] [@ubreu/homebridge-helios-kwl] connection error Error: getaddrinfo ENOTFOUND http
    at GetAddrInfoReqWrap.onlookupall [as oncomplete] (node:dns:118:26) {
  errno: -3008,
  code: 'ENOTFOUND',
  syscall: 'getaddrinfo',
  hostname: 'http'
}
UnhandledPromiseRejection: This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason "connection error".
[27/03/2024, 07:46:32] [@ubreu/homebridge-helios-kwl] Child bridge process ended
[27/03/2024, 07:46:32] [@ubreu/homebridge-helios-kwl] Process Ended. Code: 1, Signal: null

Please consider using a try/catch in this function to catch errors that are thrown and log them without crashing. Something like this:

async addAccessory(config: HeliosVentilationPlatformConfig) {
  try {
    this.hv = new HeliosVentilation(config.heliosHost, config.heliosPort, this.log);
    ...
  } catch (error) {
    this.log.error(`An error occurred: ${error.message}`);
  }
}

@ubreu
Copy link
Author

ubreu commented Mar 27, 2024

@bwp91 Thanks for your feedback! Good points, I created a new release.

@bwp91
Copy link
Contributor

bwp91 commented Mar 28, 2024

/check

@github-actions github-actions bot added pending the label given to a new verification/icon request and removed awaiting-changes use after review has started - awaiting user to make changes to plugin labels Mar 28, 2024
Copy link

The following pre-checks failed:

❌ More keywords apart from "homebridge-plugin" should be added in package.json.

Comment /check to run checks again.

@bwp91
Copy link
Contributor

bwp91 commented Mar 28, 2024

@ubreu please take the above as a suggestion, I have been improving the check script for future plugin verification requests and used your verification request as a test here

@bwp91
Copy link
Contributor

bwp91 commented Mar 28, 2024

@ubreu

can you ask the user in your issue ubreu/homebridge-helios-kwl#1 to update to 0.1.2 of the plugin and to post their logs after updating?

It is exactly these log entries which we are trying to avoid:

UnhandledPromiseRejection: This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason "connection error".

@bwp91 bwp91 added awaiting-user-reply use after review has started - awaiting user to reply to a comment and removed pending the label given to a new verification/icon request labels Mar 28, 2024
@ubreu
Copy link
Author

ubreu commented Mar 28, 2024

@bwp91 : the user was using an old version of the appliance which doesn't support websockets at all. That error is now caught in the try/catch block.

@bwp91 bwp91 added pending the label given to a new verification/icon request and removed awaiting-user-reply use after review has started - awaiting user to reply to a comment labels Mar 30, 2024
@bwp91
Copy link
Contributor

bwp91 commented Mar 30, 2024

/check

@github-actions github-actions bot added awaiting-changes use after review has started - awaiting user to make changes to plugin and removed pending the label given to a new verification/icon request labels Mar 30, 2024
@homebridge homebridge deleted a comment from github-actions bot Mar 30, 2024
@bwp91
Copy link
Contributor

bwp91 commented Mar 30, 2024

/check

@github-actions github-actions bot added pending the label given to a new verification/icon request awaiting-changes use after review has started - awaiting user to make changes to plugin and removed awaiting-changes use after review has started - awaiting user to make changes to plugin pending the label given to a new verification/icon request labels Mar 30, 2024
@homebridge homebridge deleted a comment from github-actions bot Mar 30, 2024
@bwp91
Copy link
Contributor

bwp91 commented Mar 30, 2024

/check

@github-actions github-actions bot added pending the label given to a new verification/icon request and removed awaiting-changes use after review has started - awaiting user to make changes to plugin labels Mar 30, 2024
Copy link

🟢 The following pre-checks passed:

  • Installation: successfully installed
  • Package JSON: repository.url property exists
  • Package JSON: bugs.url exists
  • Package JSON: 'preinstall' in scripts is not present
  • Package JSON: 'install' in scripts is not present
  • Package JSON: 'postinstall' in scripts is not present
  • Package JSON: engines.node property is compatible with Node 18
  • Package JSON: engines.node property is compatible with Node 20
  • Package JSON: engines.homebridge property is compatible with Homebridge 1.7.0
  • Package JSON: initializer function found
  • GitHub Repo: repository is public
  • GitHub Repo: repository is not archived
  • GitHub Repo: issues are enabled
  • GitHub Repo: contains releases
  • NPM Package: has not been deprecated
  • Config Schema JSON: exists and is valid JSON
  • Config Schema JSON: contains a valid pluginAlias
  • Config Schema JSON: the pluginType is set to 'platform'
  • Dependencies: homebridge was not installed as a dependency
  • Dependencies: hap-nodejs was not installed as a dependency

🔴 The following pre-checks failed:

  • Package JSON: more keywords apart from 'homebridge-plugin' should exist

⚠️ Please action these failures and then comment /check to run the checks again. Let us know if you need any help.

@github-actions github-actions bot added awaiting-changes use after review has started - awaiting user to make changes to plugin and removed pending the label given to a new verification/icon request labels Mar 30, 2024
@bwp91 bwp91 added verified use when a plugin meets the criteria - adds the verified badge text and removed awaiting-changes use after review has started - awaiting user to make changes to plugin labels Mar 30, 2024
Copy link

Congratulations! Your plugin has been verified!

You can now add one of the Verified by Homebridge badges 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)

verified-by-homebridge

[![verified-by-homebridge](https://img.shields.io/badge/homebridge-verified-blueviolet?color=%23491F59&style=for-the-badge&logoColor=%23FFFFFF&logo=homebridge)](https://github.com/homebridge/homebridge/wiki/Verified-Plugins)

Your plugin is now also eligible to display a ❤️ Donate button on its tile in the Homebridge UI. See https://github.com/homebridge/homebridge/wiki/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.

Don't forget to join the official Homebridge Discord server, where plugin developers can get tips and advice from other developers and the Homebridge project team in the #plugin-development channel!

As a verified plugin, you can request a channel in the Discord server to discuss your plugin with users and other developers. Just ask in the #plugin-development channel.

Thank you for your contribution to the Homebridge community.

  • The Homebridge Team

@bwp91
Copy link
Contributor

bwp91 commented Mar 30, 2024

Hi @ubreu

thanks for making the changes to your plugin. i have mark it as verified now!

Please consider adding more keywords to your package json file.

@bwp91 bwp91 closed this as completed Mar 30, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
verified use when a plugin meets the criteria - adds the verified badge text
Projects
None yet
Development

No branches or pull requests

2 participants