-
Notifications
You must be signed in to change notification settings - Fork 130
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
Remove unused code #465
Remove unused code #465
Conversation
gateway = Gateway() | ||
|
||
cmd = ( | ||
SmartTask(gateway, TASK).start_action.devices[0].item_controller.set_dimmer(30) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only place where I have seen devices
been used. I feel it's based on an incorrect assumption that you would want to control a device from a smart task.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is calling the set_dimmer
method in the smart_task module. But it looks like you can do the same via task_control.tasks
. Is there a difference in what StartActionItem
instances are enumerated in the two different lists?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest, I can't remember what caused me to go for this ambiguous design pattern. The order seem to have been the same in both occurances as we loop over self.raw.root_start_action
:
TaskControl:
return [
StartActionItem(self._task, idx, self.state, self.path, self.raw)
for idx in range(len(self.raw.root_start_action))
]
StartAction:
[
StartActionItem(self.smart_task, i, self.state, self.path, self.raw)
for i in range(len(self.raw.root_start_action))
]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then I think we can remove this as there's an alternative in place.
Closes #463