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

Child device support #2

Merged
merged 3 commits into from Mar 17, 2020
Merged

Child device support #2

merged 3 commits into from Mar 17, 2020

Conversation

adamkempenich
Copy link
Contributor

  • Added child devices for relays and backlight
  • Added logDescriptionText and logDebugText options
  • Added importURL
  • Added option to turn screen on when proximity falls below a value

- Added child devices for relays and backlight
- Added logDescriptionText and logDebugText options
- Added importURL
- Added option to turn screen on when proximity falls below a value
- Added option to turn on the above option
@joshualyon
Copy link
Owner

joshualyon commented Mar 5, 2020

Hey @adamkempenich did you get the notifications for the minor comments I added to the PR? If not, I can go ahead and just merge the PR as-is since the suggestions were really minor about uncommenting the metadata definitions for the various toggle commands that you filled in.

Edit: Whoops. I might have just discarded them:

image

Copy link

@WBEVAN WBEVAN left a comment

Choose a reason for hiding this comment

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

Code looks and works good. Couple of minor comments

May be add in
capability "Configuration"

Then from the configure method you should be able to check to see if the child devices exist and if not create them like what you have done in the installed method. This should add them to an existing device in that case. Also on the labels for the component may be preference them with Wink Relay, this way they should be in the same location as the Wink in the dashboard list.

Also the proximity logic should support a delay capability. That way the code does nto cause the device to flash on and off.

def checkProximity(){
if(device.currentValue('proximity') >= settings.screenOnProximity){
screenBacklightOn()
} else{ screenBacklightOff() }
Copy link

Choose a reason for hiding this comment

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

It might be good here to place a delay on this action. This would be in line with the original wink. At the moment this seems to flash on and off so the logic could benefit from a delay capability on here.

@adamkempenich
Copy link
Contributor Author

Hey @adamkempenich did you get the notifications for the minor comments I added to the PR? If not, I can go ahead and just merge the PR as-is since the suggestions were really minor about uncommenting the metadata definitions for the various toggle commands that you filled in.

Edit: Whoops. I might have just discarded them:

image

I've been mostly AFK the last week or so. I will add a configure/child device check, along with the delay, and resubmit.

Copy link
Contributor Author

@adamkempenich adamkempenich left a comment

Choose a reason for hiding this comment

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

Added configuration() command to create child devices

@joshualyon joshualyon merged commit 15a71af into joshualyon:master Mar 17, 2020
@joshualyon
Copy link
Owner

Thanks for your contribution @adamkempenich 👍

I've gone ahead and merged the PR.

@adamkempenich
Copy link
Contributor Author

Thank you!

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

3 participants