homebridge-plugins / homebridge-roomba2

Homebridge plugin to connect iRobot Roomba devices with Homebridge/HomeKit.
MIT License
146 stars 17 forks source link

Clean Specific Rooms Behaviour #121

Closed boomseklecki closed 1 year ago

boomseklecki commented 2 years ago

Hi, this is my first pull request. I've been using your homebridge-roomba2 package for several years and created a custom index.js file for the earlier version but when v1.3 came out I wanted to properly implement cleanRoom with arguments so others could use it. I've added a cleanBehaviour setting to choose to clean everywhere or just specific rooms, config settings for the map and region values necessary to provide an argument to cleanRoom and a javascript file to log the lastCommand properties to console so users can enter them in the settings ui. I was able to build and test the changes and I created a changeset in pnpm but I don't see it on github. I'd appreciate any feedback you might have. Thanks for your consideration.

kindofausername commented 2 years ago

Hi, is there a way to install the Plugin and including your Pull Request on Homebridge? Would love to have this feature.

boomseklecki commented 1 year ago

Thanks for the feedback. I believe I fixed all the indentation issues, sorry my editor wasn't showing them as different. I like the idea of doing a beta release since I only have two Roombas to test on I might not have considered something that affects other robots. I discovered two new parameters being returned by my getlastcommand script while I was testing, 'noAutoPasses' and 'twoPasses' both optional boolean parameters that can be passed for each cleaning region so I added them to the config.schema. I also created an interface for the 'mission' object as you suggested and added some documentation to the readme file. When I was merging your latest updates with my repository I inadvertently created a new branch. It looks like it linked this pull request to the new branch but I haven't figured out how to merge it back to main while preserving the pull request and I think changesets does not like that its a branch (the error I'm getting says 'failed to find where HEAD diverged from main') so I have not been able to create a changeset. Just wanted to call it out in case its an issue.

jlindsayDev commented 1 year ago

Hey there! I'm also interested in this feature. Are there any other changes requested, @karlvr? I am happy to contribute as well if needed.

donavanbecker commented 1 year ago

I can make a beta branch and merge this into it ti publish to npm if anyone is interested in testing?

kangolo commented 1 year ago

@donavanbecker I can test this out, I have an i3+ that has room mapping (with a single map).

@boomseklecki .DS_Store shouldn't be included in the PR, maybe add that to .gitignore.

boomseklecki commented 1 year ago

@kangolo .DS_Store was in my .gitignore so not sure when it was added. The file has been deleted

Screenshot 2023-03-02 at 8 01 35 PM
karlvr commented 1 year ago

@boomseklecki thank you very much for this, I have merged it in and committed a few minor changes on top. It's available to test now in v1.5.0-beta.4. Hopefully I didn't break anything in my tinkering! Thanks again.

karlvr commented 1 year ago

@kindofausername @kangolo @j000shDotCom there's a beta version available to test now if you're still keen, see https://github.com/homebridge-plugins/homebridge-roomba2/wiki/Pre-release-versions for instructions on installing pre-release versions.

kangolo commented 1 year ago

Sorry for the long delay, I set this up today and it all worked, cleaning 5 rooms.

I just used the official app to start a cleaning job with all the rooms I wanted to, then from the homebridge terminal (using the robot details from the plugin's config):

cd node_modules/homebridge-roomba2
npm run getlastcommand <robot_blid> <robot_pwd> <robot_ip_address>

And used the results to fill in the new values on the config page.

Thanks @karlvr & @boomseklecki