Timer doesn't get cancelled

Hi everyone,

I’ve created an app to control my lights based on motion detectors. Everything is working as intended except the timer restart does not work.

my motion_lights.py file:

import appdaemon.plugins.hass.hassapi as hass

######################################################################################
# App to turn on light when motion is detected, then turn off after a delay
# If one of the no_action_devices is on (PC, armed alarm system, AV Receiver, etc)
# a trigger of the motion sensor has no effect
# args:
#
# sensor: motion sensor to use as trigger
# light_entities: light to turn on/off, multiple lights separated by comma
# lux_sensor: when the lux of this sensor is above lux_threshold then light is not turned on
# lux_threshold: value above which light will not be turned on, default 100 lux
# timer_sec: time in seconds until light turns off when there's no motion, default 30 seconds
# no_action_entities: devices which will lead to no action on motion if they are on/armed
# day_state_entity: name of the input_select which shows time of day
# brightness_level: brightness level in % for different periods of day, default 75%
#   Morgen Wochentag: 30
#   Tag Wochentag: 70
#   Nacht Wochentag:
#   Morgen Wochenende:
#   Tag Wochenende:
#   Nacht Wochenende:
# light_color: color of light for different periods of day, e.g. orange, default white
#   Morgen Wochentag: orange
#   Tag Wochentag: white
#   Nacht Wochentag:
#   Morgen Wochenende:
#   Tag Wochenende:
#   Nacht Wochenende:
######################################################################################

class MotionLights(hass.Hass):

    def initialize(self):
        self.handle = None

        if "brightness_level" in self.args:
            self.brightness_map = self.args["brightness_level"]

        if "light_color" in self.args:
            self.color_map = self.args["light_color"]

        if "timer_sec" in self.args:
            self.delay = self.args["timer_sec"]
        else:
            self.delay = 30 # default delay

        if "sensor" in self.args:
            self.listen_state(self.motion, self.args["sensor"])
        else:
            self.log("No motion sensor specified, doing nothing!")

    def motion(self, entity, attribute, old, new, kwargs):
        if new == "on":
            self.log("Bewegung erkannt")
            if self.get_no_action_state() == False:
                if self.get_light_status() == True:
                    self.log("Licht ist bereits an, Timer neustarten")
                    self.cancel_timer(self.handle)
                    self.handle = self.run_in(self.light_off, self.delay)
                elif self.get_lux_status() == True:
                    self.log("Lichtstärke ist genug hoch, keine Aktion")
                else:
                    self.turn_light_on()

    def turn_light_on(self):
        if "light_entities" in self.args:
            entities = self.args["light_entities"].split(",")
            for entity in entities:
                self.turn_on(entity, brightness = self.get_brightness(), color_name = self.get_light_color())
            self.handle = self.run_in(self.light_off, self.delay)
            self.log("{} wurden eingeschaltet".format(self.args["light_entities"]))

    def get_no_action_state(self):
        no_action_state = False
        if "no_action_entities" in self.args:
            entities = self.args["no_action_entities"].split(",")
            for entity in entities:
                if self.get_state(entity) == "armed":
                    self.log("Einbrecher, keine Aktion um Alarmsystem nicht zu stören")
                    no_action_state = True
                elif self.get_state(entity) == "on" or self.get_state(entity) == "home":
                    self.log("{} ist eingeschaltet, keine Aktion".format(entity))
                    no_action_state = True
        return no_action_state

    def get_light_status(self):
        light_on = False
        if "light_entities" in self.args:
            entities = self.args["light_entities"].split(",")
            for entity in entities:
                if self.get_state(entity) == "on":
                    light_on = True
        return light_on

    def get_lux_status(self):
        lux_above_threshold = False
        if "lux_threshold" in self.args:
            lux_threshold = self.args["lux_threshold"]
        else:
            lux_threshold = 100

        if "lux_sensor" in self.args:
            if float(self.get_state(self.args["lux_sensor"])) > float(lux_threshold):
                lux_above_threshold = True
        return lux_above_threshold

    def get_brightness(self):
        brightness_level = 75
        state_of_day = self.get_state(self.args["day_state_entity"])
        if "brightness_level" in self.args:
            brightness_level = self.brightness_map[state_of_day]
            brightness_level = self.pcnt_to_brightness(brightness_level)
        return brightness_level

    def pcnt_to_brightness(self, brightness_pcnt):
        return 255 / 100 * int(brightness_pcnt)

    def get_light_color(self):
        light_color = "white"
        state_of_day = self.get_state(self.args["day_state_entity"])
        if "light_color" in self.args:
            light_color = self.color_map[state_of_day]
        return light_color

    def light_off(self, kwargs):
        if "light_entities" in self.args:
            entities = self.args["light_entities"].split(",")
            for entity in entities:
                self.turn_off(entity)
            self.log("{} wurden durch Timer ausgeschaltet".format(self.args["light_entities"]))

The delay for the timer is set to 900 seconds (15 min) in the configuration file.

Below is a log from when the problem occured.

2018-08-08 06:46:03.155651 INFO Ankleidezimmer Bewegung: Bewegung erkannt
2018-08-08 06:46:03.921477 INFO Ankleidezimmer Bewegung: light.ankleidezimmer wurden eingeschaltet
2018-08-08 07:01:46.546190 INFO Ankleidezimmer Bewegung: Bewegung erkannt
2018-08-08 07:01:46.546790 INFO Ankleidezimmer Bewegung: Licht ist bereits an, Timer neustarten
2018-08-08 07:02:00.379015 INFO Ankleidezimmer Bewegung: light.ankleidezimmer wurden durch Timer ausgeschaltet

As you can see the light was turned on by motion at 06:46. At 07:01 the app recognized that the light is already on so it should restart the timer, however a few seconds later at 07:02 (15 min after initial trigger) the light turns off but it should not because the timer should have been reset.

I have no clue what I’m doing wrong. Your help would be highly appreciated!

you start the timer on 2 places when the “lichstärke zu niedrig ist”.
in motion and in light_on

both timers are started with the same handle variable and you reset only 1.

I don’t understand. The first time when motion was detected it goes to turn_light_on() and starts the timer. The second time when motion was detected, it sees that the light is already on, so it cancels the timer and then starts a new one or not?

sorry i did read it wrong because you have a (for me) strangeway of starting things :wink:

the way i always use it is like:

if self.handle != None:
    self.cancel_timer(self.handle)
self.handle = self.run_in(...)

i think you can get errors out of this way of working. (did you check your error log)
if you turn on a light and motion is detected after that, you try to cancel a timer that doesnt exist at that time.
if the cancel_timer errors the run_in wont be started.
try to put your logging behind your actions and not before like

                    self.cancel_timer(self.handle)
                    self.handle = self.run_in(self.light_off, self.delay)
                    self.log("Licht ist bereits an, Timer neustarten")

if your logging isnt shown (in the logfile), you know there is an error (in your errorlog).
by the way

                if self.get_light_status() == True:

is the same as:

                if self.get_light_status():

and

            if self.get_no_action_state() == False:

is the same as:

            if not self.get_no_action_state():

Ah, I understand now. Thanks.

Thanks also for the code suggestions, you see I’m still very new to this :slight_smile:

What do you think about my programming style, are there any better/easier ways to do this task?
I’m trying to improve my coding as much as I can.

there are some things i noticed:
1)

        if "brightness_level" in self.args:
            self.brightness_map = self.args["brightness_level"]

if you dont set the brightness in the args, the var wont be filled at all and calling it elsewhere will give an error. (but you check for it again in your functions, which isnt needed when you make sure they exist)
so set a default is such cases.

you can save 1 line by doing it like:

        self.delay = 30 # default delay
        if "timer_sec" in self.args:
            self.delay = self.args["timer_sec"]
  1. like i said, i would restart the timer based on if the timer exist and not based on the lightstate
  2. i would move the timer start from the turn_light on to the motion and i wouldnt mix if statements like:
            if not self.get_lux_status():
                if self.handle != None:
                    self.cancel_timer(self.handle)
                    self.handle = self.run_in(self.light_off, self.delay)
                    self.log("Licht ist bereits an, Timer neustarten")
                else:
                    self.turn_light_on()
                    self.handle = self.run_in(self.light_off, self.delay)

so a few small things can be done, but other then that i dont see much that can give big improvements.

About code style:

            if "brightness_level" in self.args:
                self.brightness_map = self.args["brightness_level"]
            if "light_color" in self.args:
                self.color_map = self.args["light_color"]
            self.delay = 30 # default delay
            if "timer_sec" in self.args:
                self.delay = self.args["timer_sec"]

you can write shorter

            self.brightness_map = self.args.get("brightness_level", {})  # default: empty dict
            self.color_map = self.args.get("light_color")  # default: None, maybe also should be dict
            self.delay = self.args.get("timer_sec", 30)  # default: 30

and in def get_brightness(self):

            brightness_level = self.brightness_map[state_of_day]
            brightness_level = self.pcnt_to_brightness(brightness_level)

to

            brightness_level = self.pcnt_to_brightness(self.brightness_map.get(state_of_day, 0))  # default brightness_level?
3 Likes

Thanks for your suggestions, I will implement them in my code and post the full code once I’m done.
Also thanks to @adyachenko for showing me pythons .get method.

I think the code in your 3. suggestion will not work in the scenario below.

  1. motion is detected and no timer is set–> turn lights on and start a timer for let’s say 5 min
  2. person leaves room and turns light off manually
  3. motion is detected again before timer ended --> timer is restarted but the lights will not turn on and person stays in the dark

This means I need to check for the light status or have a function that cancels the timer when the light is manually turned off. Is this correct?

correct.
you need to add a listen_state for the lights, and when the lights are turned off the timer gets cancelled.

thats the only way to avoid that you try to restart the timer when it doesnt exist.

So I decided to create a separate function for the timer called handle_timer.
I tried to incorporate all your changes and this is my final working result.

import appdaemon.plugins.hass.hassapi as hass

######################################################################################
# App to turn on light when motion is detected, then turn off after a delay
# If one of the no_action_devices is on (PC, armed alarm system, AV Receiver, etc)
# a trigger of the motion sensor has no effect
# args:
#
# sensor: motion sensor to use as trigger
# light_entities: light to turn on/off, multiple lights separated by comma
# lux_sensor: when the lux of this sensor is above lux_threshold then light is not turned on
# lux_threshold: value above which light will not be turned on, default 100 lux
# timer_sec: time in seconds until light turns off when there's no motion, default 30 seconds
# no_action_entities: devices which will lead to no action on motion if they are on/armed
# day_state_entity: name of the input_select which shows time of day
# brightness_level: brightness level in % for different periods of day, default 75%
#   Morgen Wochentag: 30
#   Tag Wochentag: 70
#   Nacht Wochentag:
#   Morgen Wochenende:
#   Tag Wochenende:
#   Nacht Wochenende:
# light_color: color of light for different periods of day, e.g. orange, default white
#   Morgen Wochentag: orange
#   Tag Wochentag: white
#   Nacht Wochentag:
#   Morgen Wochenende:
#   Tag Wochenende:
#   Nacht Wochenende:
######################################################################################

class MotionLights(hass.Hass):

    def initialize(self):
        self.handle = None
        self.brightness_map = self.args.get("brightness_level", {})
        self.color_map = self.args.get("light_color", {})
        self.delay = self.args.get("timer_sec", 30) # 30 sec default delay

        if "sensor" in self.args:
            self.listen_state(self.motion, self.args["sensor"])
        else:
            self.log("Kein Bewegungssensor konfiguriert, keine Aktion!")

    def motion(self, entity, attribute, old, new, kwargs):
        if new == "on":
            self.log("Bewegung erkannt")
            if not self.get_no_action_state():
                if self.get_light_status():
                    self.log("Licht ist bereits an, Timer neustarten")
                    self.handle_timer()
                elif self.get_lux_status():
                    self.log("Lichtstärke ist genug hoch, keine Aktion")
                else:
                    self.turn_light_on()
                    self.handle_timer()

    def turn_light_on(self):
        if "light_entities" in self.args:
            entities = self.args["light_entities"].split(",")
            for entity in entities:
                self.turn_on(entity, brightness = self.get_brightness(), color_name = self.get_light_color())
            self.log("{} wurden eingeschaltet".format(self.args["light_entities"]))
            self.log("Helligkeit {} %, Lichtfarbe {}".format(self.get_brightness(), self.get_light_color()))

    def handle_timer(self):
        if self.handle != None:
            self.cancel_timer(self.handle)
            self.log("Timer angehalten")
        self.handle = self.run_in(self.light_off, self.delay)
        self.log("Timer für {} Sekunden eingeschaltet".format(self.delay))

    def get_no_action_state(self):
        no_action_state = False
        if "no_action_entities" in self.args:
            entities = self.args["no_action_entities"].split(",")
            for entity in entities:
                if self.get_state(entity) == "armed":
                    self.log("Einbrecher, keine Aktion um Alarmsystem nicht zu stören")
                    no_action_state = True
                elif self.get_state(entity) == "on" or self.get_state(entity) == "home":
                    self.log("{} ist eingeschaltet, keine Aktion".format(entity))
                    no_action_state = True
        return no_action_state

    def get_light_status(self):
        light_on = False
        if "light_entities" in self.args:
            entities = self.args["light_entities"].split(",")
            for entity in entities:
                if self.get_state(entity) == "on":
                    light_on = True
        return light_on

    def get_lux_status(self):
        lux_above_threshold = False
        lux_threshold = self.args.get("lux_threshold", 100)
        if "lux_sensor" in self.args:
            if float(self.get_state(self.args["lux_sensor"])) > float(lux_threshold):
                lux_above_threshold = True
        return lux_above_threshold

    def get_brightness(self):
        state_of_day = self.get_state(self.args["day_state_entity"])
        brightness_level = self.pcnt_to_brightness(self.brightness_map.get(state_of_day, 75))
        return brightness_level

    def pcnt_to_brightness(self, brightness_pcnt):
        return 255 / 100 * int(brightness_pcnt)

    def get_light_color(self):
        state_of_day = self.get_state(self.args["day_state_entity"])
        light_color = self.color_map.get(state_of_day, "white")
        return light_color

    def light_off(self, kwargs):
        if "light_entities" in self.args:
            entities = self.args["light_entities"].split(",")
            for entity in entities:
                self.turn_off(entity)
            self.log("{} wurden durch Timer ausgeschaltet".format(self.args["light_entities"]))

I will remove all the logs after testing the app for a while. Is this a good practice or should I rather leave the logs?

there are several options for that.
you could let them in as long as they dont annoy you in the logs.
you could let them in with an # before it so that you can get them back when you want (thats what i tend to do)
or you could create an option that the logs are only shown when you put “log: True” in the yaml
i do that when i share apps, so others can easyly debug.
to do that you put

self.log = self.args.get("log", False)

in the initialise and then you put this line:

if self.log:

in front of every logline.

i think you did a good job!