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

feat(notification): add simple SmartThings switch activation #1902

Merged
merged 22 commits into from
Feb 7, 2021
Merged

feat(notification): add simple SmartThings switch activation #1902

merged 22 commits into from
Feb 7, 2021

Conversation

pmilano1
Copy link
Contributor

@pmilano1 pmilano1 commented Feb 6, 2021 •
edited
Loading

Description

Added basic switch activation for SmartThings, will turn on switch based on the below env vars. Motivation being I work in the dark, and wanted to turn a light on as a notification.

SMARTTHINGS_TOKEN=
SMARTTHINGS_SWITCH_LABEL=

Testing

  • Tested with each of the above vars empty - no activation attempt
  • Tested with each of the above incorrect - warning messages about incorrect var
  • Tested with each of the above correct - on come the lights!

New dependencies

  • "@bridgerakol/samsung-smart-api": "^2.8.1"

@pmilano1 pmilano1 requested a review from jef as a code owner February 6, 2021 21:41
@jef
Copy link
Owner

jef commented Feb 7, 2021

Added so that linting works properly on windows.

I've been wanting to know how to fix this! Thank you.

@pmilano1
Copy link
Contributor Author

pmilano1 commented Feb 7, 2021

Added so that linting works properly on windows.

I've been wanting to know how to fix this! Thank you.

Ideally I would have done it prior to my horrible commit chain - but I felt challenged by it :)

jef
jef reviewed Feb 7, 2021
View reviewed changes
Copy link
Owner

@jef jef left a comment

Choose a reason for hiding this comment

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

Overall looks great! Thanks for the implementation. Just left a few comments if you don't mind addressing ☺️

src/notification/smartthings.ts Outdated Show resolved Hide resolved
src/notification/smartthings.ts Outdated Show resolved Hide resolved
pmilano1 and others added 4 commits February 7, 2021 14:15
jef
jef approved these changes Feb 7, 2021
View reviewed changes
Copy link
Owner

@jef jef left a comment

Choose a reason for hiding this comment

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

Awesome! Lets pull this in. Thanks for everything!

@jef jef changed the title feat: added simple smartthings switch activation feat(notification): add simple SmartThings switch activation Feb 7, 2021
@jef jef merged commit c22c960 into jef:main Feb 7, 2021
@pmilano1
Copy link
Contributor Author

Let it be known that I landed a 3090 on day two with this :)

@jef
Copy link
Owner

jef commented Feb 10, 2021

YAY!!!

erwinc1 pushed a commit to erwinc1/streetmerchant that referenced this pull request Mar 31, 2021
@pmilano1 @jef @erwinc1
feat(notification): add simple SmartThings switch activation (jef#1902)
58d0601 
Co-authored-by: Jef LeCompte <jeffreylec@gmail.com>
@github-actions github-actions bot mentioned this pull request Oct 21, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jef jef jef approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pmilano1 @jef