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

"Execute commands" output doesn't turn off by itself #432

Closed
antoinechauveau opened this issue Mar 23, 2018 · 3 comments
Closed

"Execute commands" output doesn't turn off by itself #432

antoinechauveau opened this issue Mar 23, 2018 · 3 comments

Comments

@antoinechauveau
Copy link
Contributor

Mycodo Issue Report:

Mycodo Version: 5.6.6

Problem Description

When working with a "Execute commands" output, Mycodo turns this output on just fine but doesn't turn it off when appropriate.

Steps to reproduce the issue:

This is reproducible directly on the "Output" page:

  • Add a "Execute commands" output with its "On" and "Off" commands (can be configured as "echo" commands to a file in case nothing better is available for testing).
  • Type in a "Duration On" and click the test "Turn On" button.
  • Observe that the output turns on, but fails to turn off after the requested duration.

Root cause and proposed fix:

Mycodo checks whether the output should be turned off with the following code snippet in controller_output.py:

if (self.output_on_until[output_id] < current_time and
    self.output_on_duration[output_id] and
    self.output_pin[output_id] is not None):

The issue is that "Execute commands" outputs aren't associated to a pin. So the above condition never becomes true.

I get the expected behavior with the following fix:

if (self.output_on_until[output_id] < current_time and
    self.output_on_duration[output_id] and
    (self.output_type[output_id] not in [
         'pwm', 'wired', 'wireless_433MHz_pi_switch'] or
    self.output_pin[output_id] is not None)):
@kizniche
Copy link
Owner

Thanks. I'll add this fix to the next release. It's good to see a bug report with an accompanying fix!

@antoinechauveau
Copy link
Contributor Author

Thanks for looking into this quickly Kyle, unfortunately I just noticed that the patch you submitted has the condition implemented backwards, so it doesn't work.

The code is now:

if (self.output_on_until[output_id] < current_time and
        self.output_on_duration[output_id] and
        ('command' not in self.output_type[output_id] or
         self.output_pin[output_id] is not None)):

But it should be:

if (self.output_on_until[output_id] < current_time and
        self.output_on_duration[output_id] and
        ('command' in self.output_type[output_id] or
         self.output_pin[output_id] is not None)):

@kizniche
Copy link
Owner

Whoops! Thanks for catching that. I'll fix it for the next release.

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

No branches or pull requests

2 participants