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

Remove unused code #465

Merged
merged 4 commits into from
Mar 19, 2022
Merged

Remove unused code #465

merged 4 commits into from
Mar 19, 2022

Conversation

ggravlingen
Copy link
Member

Closes #463

gateway = Gateway()

cmd = (
SmartTask(gateway, TASK).start_action.devices[0].item_controller.set_dimmer(30)
Copy link
Member Author

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.

Copy link
Contributor

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?

Copy link
Member Author

@ggravlingen ggravlingen Mar 19, 2022

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))
]

Copy link
Contributor

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.

@ggravlingen ggravlingen merged commit 14f8cdd into master Mar 19, 2022
@ggravlingen ggravlingen deleted the remove-code branch March 19, 2022 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants