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

homebridge-multiswitcheroo #562

Closed
iSteve-O opened this issue Sep 1, 2023 · 18 comments
Closed

homebridge-multiswitcheroo #562

iSteve-O opened this issue Sep 1, 2023 · 18 comments
Labels
verified use when a plugin meets the criteria - adds the verified badge text

Comments

@iSteve-O
Copy link

iSteve-O commented Sep 1, 2023

Link To GitHub Repo

https://github.com/iSteve-O/homebridge-multiswitcheroo/tree/OffFix

Link To NPM Package

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

@iSteve-O iSteve-O added the pending the label given to a new verification/icon request label Sep 1, 2023
@github-actions
Copy link

github-actions bot commented Sep 1, 2023

✅ Pre-checks completed successfully.

@bwp91
Copy link
Contributor

bwp91 commented Sep 1, 2023

Hi @iSteve-O

Out of interest, could this plugin be considered a replacement for the already existing
https://github.com/chrszlz/homebridge-switcheroo
?

@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 Sep 1, 2023
@iSteve-O
Copy link
Author

iSteve-O commented Sep 3, 2023

Hi @bwp91

sort of but not really. Sort of in that mine does the same thing, but not really in that mine only does GET type requests currently, but does live status. Switcheroo does not have live status, but it does POST, PUT & DELETE as well.

I am slowly working on adding those other types of http requests but it does what I need already. When I do add those other types, then it will be a full replacement for Switcheroo, I think, plus live status polling.

I built this because HTTP-Switch doesn’t have multi-switches, and Switcheroo doesn’t poll for status. I only needed GET so I didn’t intend to be a full replacement at first. To be clear I can’t commit I’ll ever release a version that does those other types of requests, but I have started working on it.

Sorry for the storybook reply! Have a great day!

@bwp91
Copy link
Contributor

bwp91 commented Sep 3, 2023

Thanks!

A simple/empty config of:

    "accessories": [
        {
            "accessory": "MultiSwitcheroo",
            "name": "Test",
            "pollingInterval": 3000,
            "_bridge": {
                "username": "0E:D9:DC:A2:AD:91",
                "port": 55285
            }
        }
    ],

is crashing homebridge:

[03/09/2023, 12:45:34] Registering accessory 'homebridge-multiswitcheroo.MultiSwitcheroo'
[03/09/2023, 12:45:34] [Test] Loaded homebridge-multiswitcheroo v3.0.1 child bridge successfully

/usr/local/lib/node_modules/homebridge-multiswitcheroo/index.js:65
    for (const switchConfig of config.switches) {
                                      ^
TypeError: config.switches is not iterable
    at new MultiSwitcheroo (/usr/local/lib/node_modules/homebridge-multiswitcheroo/index.js:65:39)
    at ChildBridgeFork.startBridge (/usr/local/lib/node_modules/homebridge/src/childBridgeFork.ts:172:52)
[03/09/2023, 12:45:34] [Test] Child bridge process ended
[03/09/2023, 12:45:34] [Test] Process Ended. Code: 1, Signal: null
[03/09/2023, 12:45:42] [Test] Restarting Process...
[03/09/2023, 12:45:43] [Test] Launched child bridge with PID 7642
[03/09/2023, 12:45:43] Registering accessory 'homebridge-multiswitcheroo.MultiSwitcheroo'
[03/09/2023, 12:45:43] [Test] Loaded homebridge-multiswitcheroo v3.0.1 child bridge successfully

/usr/local/lib/node_modules/homebridge-multiswitcheroo/index.js:65
    for (const switchConfig of config.switches) {
                                      ^
TypeError: config.switches is not iterable
    at new MultiSwitcheroo (/usr/local/lib/node_modules/homebridge-multiswitcheroo/index.js:65:39)
    at ChildBridgeFork.startBridge (/usr/local/lib/node_modules/homebridge/src/childBridgeFork.ts:172:52)
[03/09/2023, 12:45:43] [Test] Child bridge process ended

Please check that config.switches is a proper array before trying to loop over it, or stop the plugin from loading further!

@iSteve-O
Copy link
Author

iSteve-O commented Sep 6, 2023

Link To GitHub Repo

https://github.com/iSteve-O/homebridge-multiswitcheroo/tree/OffFix

Link To NPM Package

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

Hey @bwp91 I just pushed new version 3.0.2 to fix this issue. I tried it with that crazy config and it no longer crashes HB. I appreciate your help!

@bwp91
Copy link
Contributor

bwp91 commented Sep 6, 2023

Screenshot 2023-09-06 at 23 54 33 why is the text so big?!

@iSteve-O
Copy link
Author

iSteve-O commented Sep 7, 2023

Screenshot 2023-09-06 at 23 54 33 why is the text so big?!

Because my original v1.0 had 2 types of accessories available. I fundamentally misunderstood that accessory plugins should only expose one accessory type, so with the 2.0 version I eliminated the “switcheroo” single switch accessory. I just wanted it to be very clear to anyone updating that they would lose the switcheroo single switch functionality and that they should instal http-switch to get it back.

@bwp91
Copy link
Contributor

bwp91 commented Sep 7, 2023

So is your plugin dynamic-platform type now?

@iSteve-O
Copy link
Author

iSteve-O commented Sep 7, 2023

So is your plugin dynamic-platform type now?

No, it’s still an accessory type plugin, but it used to have a “switcheroo” & “multiswitcheroo” accessory type. You can see the original version here. Now it only has 1 type of accessory, “multiswitcheroo”, but is still an accessory type plugin.

@bwp91
Copy link
Contributor

bwp91 commented Sep 8, 2023

When I check the link to your github plugin page in your original post, I can see that a config.schema.json is present, but when I check the main branch, this file does not exist.

One would expect that npm releases are from the main branch, is this true in your case?

@iSteve-O
Copy link
Author

iSteve-O commented Sep 9, 2023

When I check the link to your github plugin page in your original post, I can see that a config.schema.json is present, but when I check the main branch, this file does not exist.

One would expect that npm releases are from the main branch, is this true in your case?

No, I’m sure I’m not doing this properly. The main branch was my original version, and for subsequent versions I cloned the repository and made a new branch so I’d have an image of the old working branch. I gave each branch a name for what I was working on (OffFix to fix a bug when switches were turned off, NewMethods to try to add more than GET, etc.).

I do have the package files of each version configured accordingly so the links point to the proper branch for each.

I really need to read into repository management. It’s not something I ever considered before this.

OffFix is the branch published to npm, but every branch except 1 is published as a version on npm so I’m not sure how I could correct this at this point. I can’t really unpublish the npm packages, or change the branches at this point, I don’t think.

@bwp91
Copy link
Contributor

bwp91 commented Sep 10, 2023

It looks like you are using the github releases and tagging correctly ideally for a homebridge plugin.

What one would normally do is create a feature branch (for example OffFix) and once it is ready, merge this into the main branch and create a (1) new npm release (2) github release and tag.

The github tags allow you to go back to how the code was at any time. If you are not familiar with git commands then I would definitely recommend the github desktop app.

If you are on discord I am happy to guide you towards I guess the norm.

None of these comments are meant to suggest you are doing anything wrong.

It's just that for any user who wants to install the plugin and wants to have a look at the code beforehand, they would assume that the newest npm version of the plugin would equal the main branch in the corresponding github repo

@iSteve-O
Copy link
Author

Yeah, you’re absolutely right about that and that’s everything I was doing except the final merge. I like having full working examples of the code, for now, until I’m fully comfortable using tags and all.

I have merged the latest version to the main branch, and the next time I publish an update to npm I’ll be sure to merge first so the main is always latest & fix all the links so they point back to the main.

This all makes so much more sense now and I feel kind of dumb. I was changing all the links with each release and all to point to the new branch, but if I just publish from the main I don’t have to worry about any of that. So simple and I just missed it. I thought to publish on npm the code for each version had to be on GitHub, but that doesn’t make any sense now that I think about it.

Anyway, I understand, have done it, and will do going forward. Main is main. I really appreciate you taking the time & offering to help.

@bwp91
Copy link
Contributor

bwp91 commented Sep 10, 2023

When I install the plugin again and immediately click save on the plugin settings screen that appears, I get this message in the log:

[10/09/2023, 23:54:54] Could not load accessory MultiSwitcheroo at position 1 as it is missing the required 'name' property!

I guess this is because there is no default value for the name field

Screenshot 2023-09-10 at 23 56 10

I wonder if it is worth having a default value in the config schema json just to avoid the plugin not loading for users rushing to install and configured.


Also if the first field in the config screen must be this value, I wonder if it could just be hidden instead. And this is something to maybe consider, certainly not a job for 'today'.

I could see two options here, the first would work, the second I just thought of but I don't know if it would actually work.

@bwp91 bwp91 added currently-reviewing use for starting a review - adds a comment with the verification list with empty checkboxes reviewed and removed awaiting-user-reply use after review has started - awaiting user to reply to a comment labels Sep 10, 2023
@homebridge homebridge deleted a comment from github-actions bot Sep 10, 2023
@github-actions
Copy link

  • - The plugin must successfully install.
  • - The plugin must implement the Homebridge Plugin Settings GUI.
  • - The plugin must not start unless it is configured.
  • - 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.
  • - The plugin must not execute post-install scripts that modify the user's system in any way.
  • - The plugin must not contain any analytics or calls that enable you to track the user.
  • - The plugin must not throw unhandled exceptions, the plugin must catch and log its own errors.
  • - The plugin must be published to npm and the source code available on GitHub.
  • - A GitHub release should be created for every new version of your plugin, with patch notes.
  • - The plugin must run on all Active LTS versions of Node.js, at the time of writing this is Node.js v18.
  • - The plugin must not require the user to run Homebridge in a TTY or with non-standard startup parameters, even for initial configuration.
  • - If the plugin needs to write files to disk (cache, keys, etc.), it must store them inside the Homebridge storage directory.

Everything Looks Good!

@bwp91 bwp91 added verified use when a plugin meets the criteria - adds the verified badge text and removed currently-reviewing use for starting a review - adds a comment with the verification list with empty checkboxes reviewed labels Sep 10, 2023
@github-actions
Copy link

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 ❤️ 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!

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

@iSteve-O
Copy link
Author

When I install the plugin again and immediately click save on the plugin settings screen that appears, I get this message in the log:

[10/09/2023, 23:54:54] Could not load accessory MultiSwitcheroo at position 1 as it is missing the required 'name' property!

I guess this is because there is no default value for the name field

Screenshot 2023-09-10 at 23 56 10

I wonder if it is worth having a default value in the config schema json just to avoid the plugin not loading for users rushing to install and configured.

Also if the first field in the config screen must be this value, I wonder if it could just be hidden instead. And this is something to maybe consider, certainly not a job for 'today'.

I could see two options here, the first would work, the second I just thought of but I don't know if it would actually work.

Thanks Ben. I’m going to work on this.

@bwp91 bwp91 closed this as completed Sep 13, 2023
@bwp91
Copy link
Contributor

bwp91 commented Sep 13, 2023

happy homebridge-ing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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