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

Add set_repeat_days to SmartTask #615

Conversation

blast-hardcheese
Copy link
Contributor

@blast-hardcheese blast-hardcheese commented Jan 16, 2023

  • Added Days with constants into const.py
  • Added TaskControl.set_repeat_days for SmartTask
  • Added lights to the onboarding helper text
  • Added sockets and socket to __main__
  • Added an example text for how to use set_repeat_days in __main__

@blast-hardcheese
Copy link
Contributor Author

In response to "We don't use the const module above. I think we should be consistent",

I think having symbolic names for the individual days is critical as a disambiguation, since "0b1000000 is Monday" isn't obvious, and avoiding trial and error seems wise

@blast-hardcheese
Copy link
Contributor Author

Actually, I didn't see WEEKDAYS initially. If there's some other way you'd like me to approach this, I'm happy to do so

Comment on lines +142 to +144
print("> from pytradfri.const.Days import TUESDAY, THURSDAY, SATURDAY, SUNDAY")
print("> days = TUESDAY | THURSDAY | SATURDAY | SUNDAY")
print("> api(socket.socket_control.set_repeat_days(days))")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would avoid the modification to const.py:

Suggested change
print("> from pytradfri.const.Days import TUESDAY, THURSDAY, SATURDAY, SUNDAY")
print("> days = TUESDAY | THURSDAY | SATURDAY | SUNDAY")
print("> api(socket.socket_control.set_repeat_days(days))")
print("> from pytradfri.smart_task import WEEKDAYS")
print("> days = WEEKDAYS.tue | WEEKDAYS.thur | WEEKDAYS.sat | WEEKDAYS.sun")
print("> api(socket.socket_control.set_repeat_days(days))")

Copy link
Contributor

Choose a reason for hiding this comment

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

Please use the existing constant in smart_task.

@github-actions github-actions bot added the Stale label Feb 23, 2023
@github-actions github-actions bot closed this Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants