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.

Sign up for GitHub

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

Jump to bottom

[irobot] iRobot cleaning zone support #11413

Closed
wants to merge 15 commits into from
Closed

[irobot] iRobot cleaning zone support #11413

wants to merge 15 commits into from

Conversation

Nuesel
Copy link
Contributor

@Nuesel Nuesel commented Oct 19, 2021

Hello,
I bought a new iRobot Roomba i7. But I encountered the problem, the robot couldn't clean rooms nor zones defined in the iRobot app. See: #11340
I tested the code with the iRobot firmware 3.18.11 (currently up to date). The changes seem to work fine.

Regards,
Nuesel

@kaikreuzer kaikreuzer added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Oct 21, 2021
@Nuesel
Added type zone to iRobot
3e96904 
Signed-off-by: Nuesel <nuesel@gruenbaer.net>
@wborn wborn added the enhancement An enhancement or new feature for an existing add-on label Nov 27, 2021
lolodomo
lolodomo requested changes Dec 11, 2021
View reviewed changes
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just few corrections need to be better compliant with naming conventions.

...enhab.binding.irobot/src/main/java/org/openhab/binding/irobot/internal/dto/MQTTProtocol.java Outdated Show resolved Hide resolved
....binding.irobot/src/main/java/org/openhab/binding/irobot/internal/handler/RoombaHandler.java Outdated Show resolved Hide resolved
....binding.irobot/src/main/java/org/openhab/binding/irobot/internal/handler/RoombaHandler.java Outdated Show resolved Hide resolved
....binding.irobot/src/main/java/org/openhab/binding/irobot/internal/handler/RoombaHandler.java Outdated Show resolved Hide resolved
....binding.irobot/src/main/java/org/openhab/binding/irobot/internal/handler/RoombaHandler.java Outdated Show resolved Hide resolved
bundles/org.openhab.binding.irobot/README.md Outdated
```

The easiest way to determine the pmapId and region_ids is to monitor the last_command channel while starting a new mission for the specific region with the iRobot-App.
Some devices support cleaning rooms (aka regions). Additionally, support for cleaning rectangle areas previously defined in the iRobot-App (aka zones) may be available. If the type string such as `r=` (region) or `z=` (zone) is omnitted, the type defaults to region.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a cariage return after each sentence.

lolodomo
lolodomo previously approved these changes Dec 11, 2021
View reviewed changes
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Nuesel
Revert "Merge branch 'openhab:main' into main"
4e152f1 
This reverts commit 5ebba0b, reversing
changes made to 496a1d3.
@lolodomo lolodomo dismissed their stale review December 11, 2021 19:11

Changes after approval

@Nuesel
Copy link
Contributor Author

Nuesel commented Dec 11, 2021

It seems I have some trouble with git. I will dig into tomorrow again.

@lolodomo
Copy link
Contributor

lolodomo commented Dec 11, 2021 •
edited
Loading

This is now a disaster after your last merge action.
Please update your main branch, create a new branch, reapply your changes, commit with proper sign-off and finally create a new PR.

@Nuesel
Copy link
Contributor Author

Nuesel commented Dec 12, 2021

This is now a disaster after your last merge action. Please update your main branch, create a new branch, reapply your changes, commit with proper sign-off and finally create a new PR.

My last action reverted a merge with the addons mainline.
But somehow, my fork is jinxed. If I compare the changes in my fork to the mainline, I see many files I haven't touched:
main...Nuesel:main
Is it a good idea to revert all changes and start from scratch? What can go wrong to run in such a mess?

@lolodomo
Copy link
Contributor

Make a copy of your few changed files for this PR.
Update your main branch, create a new local branch with these files.
Finally create a new clean PR.

@Nuesel
Copy link
Contributor Author

Nuesel commented Dec 12, 2021

My changes are still in my forks's main. I've just reverted a merge with Openhab addons main. Now, I just merged with Openhab addons main, again.
This was why I asked whether to start from scratch again (make a new fork and branch). I think I will do so in the next days.
In any case, thanks for your help.

@Nuesel Nuesel mentioned this pull request Dec 14, 2021
Merged
@Nuesel
Copy link
Contributor Author

Nuesel commented Dec 14, 2021

I close this pull request in favour of PR #11783 that replaces this one.

@Nuesel Nuesel closed this Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlaur jlaur jlaur left review comments

@lolodomo lolodomo lolodomo left review comments

@clinique clinique Awaiting requested review from clinique clinique is a code owner

@computergeek1507 computergeek1507 Awaiting requested review from computergeek1507

@Confectrician Confectrician Awaiting requested review from Confectrician

@cweitkamp cweitkamp Awaiting requested review from cweitkamp

@davidgraeff davidgraeff Awaiting requested review from davidgraeff

@fazioa fazioa Awaiting requested review from fazioa

@fruggy83 fruggy83 Awaiting requested review from fruggy83

@Hilbrand Hilbrand Awaiting requested review from Hilbrand

@hmerk hmerk Awaiting requested review from hmerk

@Jamstah Jamstah Awaiting requested review from Jamstah

@kaikreuzer kaikreuzer Awaiting requested review from kaikreuzer

@kgoderis kgoderis Awaiting requested review from kgoderis kgoderis is a code owner

@lujop lujop Awaiting requested review from lujop

@markus7017 markus7017 Awaiting requested review from markus7017

@cschneider cschneider Awaiting requested review from cschneider cschneider is a code owner

@digitaldan digitaldan Awaiting requested review from digitaldan

@gerrieg gerrieg Awaiting requested review from gerrieg

@jochen314 jochen314 Awaiting requested review from jochen314 jochen314 is a code owner

Assignees
No one assigned
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@Nuesel @hmerk @lolodomo @jlaur @kaikreuzer @wborn