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

Feature/shutter control #2

Merged
merged 8 commits into from Jun 15, 2020

Conversation

coeing
Copy link
Collaborator

@coeing coeing commented Jun 9, 2020

TODO:

  • Stopping not working as implemented, check if possible or remove
  • Add PercentType command to set precise percentage of shutter level
  • Move service name "ShutterControl" into constant variable
  • Remove Gateway-ID from header of putState request

@coeing
Copy link
Collaborator Author

coeing commented Jun 11, 2020

Stopping the shutters from moving doesn't seem possible with the current API, so I created an issue for it: BoschSmartHome/bosch-shc-api-docs#34

Maybe I can find a workaround in the meantime.

Christian Oeing added 4 commits June 11, 2020 21:02
@coeing
Copy link
Collaborator Author

coeing commented Jun 11, 2020

Alright, this was a short but very successful coding session :) I handled all the remaining TODOs, so from my side the pull request is ready to merge. But please check if it is okay from your side as well!

I am using some general functions that can also be used in other places, but I would like to do the refactoring in a separate branch.

Next time I have time I will add some issues for the parts I'd like to change, so we can discuss them if necessary.

@coeing coeing marked this pull request as ready for review June 11, 2020 21:46
Copy link
Owner

@stefan-kaestle stefan-kaestle left a comment

Choose a reason for hiding this comment

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

Great work, thanks @coeing!

This all looks good to me. To suggestions:

  • Update the list of supported things in Readme.md
  • Find a place to put attributions to everybody who worked on the binding. I realize that my editor put my name in a lot of files that you (and Gerd) have been helping to improve, so maybe just add yourself there? Or remove my name and rely on the git history for that?

Great work! 🎆

Please feel free to click the merge button whenever you are ready :-)

@coeing
Copy link
Collaborator Author

coeing commented Jun 14, 2020

@stefan-kaestle Thanks for the review :)

About your points:

  • I can add some information about the new supported thing to the Readme.md, that's a good point!
  • I would suggest to have a list of contributors in the Readme.md and not in every single code file. If anybody is interested who created which code fragment, he/she could easily use Git Blame for it :)

If you give me write access to the repository, I'll merge my branch after the addition of the shutter control info to the Readme.md. Otherwise I'll let you know when I did the addition :)

@coeing
Copy link
Collaborator Author

coeing commented Jun 15, 2020

Alright, I added the Shutter Control device to the supported device section of the Readme.md and added a null reference check because on initialization the device state doesn't seems to be fetched correctly.

Will merge the pull request now! 😱

@coeing coeing merged commit cd6ac25 into stefan-kaestle:bosch-shc-2.5 Jun 15, 2020
@stefan-kaestle
Copy link
Owner

Great work! Thanks Christian! 🎉🎉🎉🎉

coeing added a commit that referenced this pull request Aug 31, 2020
Support for Bosch Shutter Control in-wall device

Signed-off-by: Christian Oeing <christian.oeing@slashgames.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants