diff --git a/appdaemon/adapi.py b/appdaemon/adapi.py index ea098276d..b74166932 100644 --- a/appdaemon/adapi.py +++ b/appdaemon/adapi.py @@ -3225,7 +3225,6 @@ async def run_every( start: str | dt.time | dt.datetime | None = None, interval: TimeDeltaLike = 0, *args, - immediate: bool = False, random_start: TimeDeltaLike | None = None, random_end: TimeDeltaLike | None = None, pin: bool | None = None, @@ -3242,7 +3241,9 @@ async def run_every( intervals will be calculated forward from the start time, and the first trigger will be the first interval in the future. - - If this is a ``str`` it will be parsed with :meth:`~appdaemon.adapi.ADAPI.parse_time()`. + - If this is ``now`` (default), then the first trigger will be now + interval + - If this is ``immediate``, then the first trigger will happen immediately + - Other ``str`` types will be parsed with :meth:`~appdaemon.adapi.ADAPI.parse_time()`. - If this is a ``datetime.time`` object, the current date will be assumed. - If this is a ``datetime.datetime`` object, it will be used as is. @@ -3258,8 +3259,6 @@ async def run_every( - If this is a ``timedelta`` object, the current date will be assumed. *args: Arbitrary positional arguments to be provided to the callback function when it is triggered. - immediate (bool, optional): Whether to immediately fire the callback or wait until the first interval if the - start time is now. random_start (int, optional): Start of range of the random time. random_end (int, optional): End of range of the random time. pin (bool, optional): Optional setting to override the default thread pinning behavior. By default, this is @@ -3313,7 +3312,7 @@ def timed_callback(self, **kwargs): ... # example callback """ interval = utils.parse_timedelta(interval) - next_period = await self.AD.sched.get_next_period(interval, start, immediate=immediate) + next_period = await self.AD.sched.get_next_period(interval, start) self.logger.debug( "Registering %s for run_every in %s intervals, starting %s", diff --git a/appdaemon/scheduler.py b/appdaemon/scheduler.py index 94a363a78..1dfd4e33d 100644 --- a/appdaemon/scheduler.py +++ b/appdaemon/scheduler.py @@ -483,22 +483,31 @@ async def get_next_period( self, interval: TimeDeltaLike, start: time | datetime | str | None = None, - *, - immediate: bool = False, ) -> datetime: + """Calculate the next execution time for a periodic interval. + + If start is "immediate", returns the current time. + Otherwise, calculates a start time (defaulting to "now") and advances by the + interval until a future time is reached. + """ interval = utils.parse_timedelta(interval) start = "now" if start is None else start # Get "now" once and use it consistently to avoid timing races now = await self.get_now() - aware_start = await self.parse_datetime(start, aware=True, now=now) - assert isinstance(aware_start, datetime) and aware_start.tzinfo is not None - - # Skip forward to the next period if start is in the past - while aware_start < now or (immediate and aware_start <= now): - aware_start += interval - - return aware_start + match start: + case "immediate": + return now + case "now", _: + aware_next = await self.parse_datetime(start, aware=True, now=now) + # Skip forward to the next period if start is in the past + # This makes the result in the first + while aware_next <= now: + aware_next += interval + + assert isinstance(aware_next, datetime) and aware_next.tzinfo is not None, \ + "aware_start must be a timezone aware datetime" + return aware_next async def terminate_app(self, name: str): if app_sched := self.schedule.pop(name, False): diff --git a/docs/HISTORY.md b/docs/HISTORY.md index 3f65f1ef6..808643c59 100644 --- a/docs/HISTORY.md +++ b/docs/HISTORY.md @@ -9,7 +9,7 @@ - Reload modified apps on SIGUSR2 - contributed by [chatziko](https://github.com/chatziko) - Using urlib to create endpoints from URLs - contributed by [cebtenzzre](https://github.com/cebtenzzre) - Added {py:meth}`~appdaemon.plugins.hass.hassapi.Hass.process_conversation` and {py:meth}`~appdaemon.plugins.hass.hassapi.Hass.reload_conversation` to the {ref}`Hass API `. -- Added `immediate` kwargs to `run_every` to control semantics around `start == "now"` +- Added special value `immediate` to {py:meth}`~appdaemon.adapi.ADAPI.run_every` semantics for the `start` kwarg. See the method docs for more information. **Fixes** diff --git a/tests/conf/apps/scheduler_test_app.py b/tests/conf/apps/scheduler_test_app.py index c7d02ae43..c51a2b0f2 100644 --- a/tests/conf/apps/scheduler_test_app.py +++ b/tests/conf/apps/scheduler_test_app.py @@ -7,9 +7,22 @@ class SchedulerTestAppMode(str, Enum): """Enum for different modes of the SchedulerTestApp.""" RUN_EVERY = "run_every" + RUN_IN = "run_in" class SchedulerTestApp(ADAPI): + """ + A test app to verify scheduler functionality. + + Configuration Args: + mode (str, optional): The mode of operation. Defaults to 'run_every'. + register_delay (float, optional): Delay before setup in seconds. Defaults to 0.5. + + RUN_EVERY: + interval (int): Interval in seconds for run_every. Required. + msg (str): Message to pass to callback. Required. + start (str, optional): Start time description. Defaults to "now". + """ def initialize(self): self.set_log_level("DEBUG") self.log("SchedulerTestApp initialized") @@ -20,10 +33,14 @@ def setup_callback(self, **kwargs) -> None: self.log(f"Running in {self.mode} mode") match self.mode: case SchedulerTestAppMode.RUN_EVERY: - start = self.args.get("start", "now") - interval = self.args["interval"] - msg = self.args["msg"] - self.run_every(self.run_every_callback, start=start, interval=interval, msg=msg) + match self.args: + case {"interval": interval, "msg": str(msg)}: + start = self.args.get("start", "now") + self.run_every(self.run_every_callback, start=start, interval=interval, msg=msg) + return + case SchedulerTestAppMode.RUN_IN: + pass + raise ValueError(f"Invalid arguments for {self.mode}") @property def mode(self) -> SchedulerTestAppMode: diff --git a/tests/conftest.py b/tests/conftest.py index 5decf26fc..65a1d4c8b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -28,7 +28,7 @@ async def ad_obj(running_loop: asyncio.BaseEventLoop, ad_cfg: AppDaemonConfig, l for cfg in ad.logging.config.values(): logger_ = logging.getLogger(cfg["name"]) logger_.propagate = True - logger_.setLevel("DEBUG") + # logger_.setLevel("DEBUG") await ad.app_management._process_import_paths() ad.app_management.dependency_manager = DependencyManager(python_files=list(), config_files=list()) diff --git a/tests/functional/test_run_every.py b/tests/functional/test_run_every.py index 9e0cf1261..056194efa 100644 --- a/tests/functional/test_run_every.py +++ b/tests/functional/test_run_every.py @@ -1,11 +1,13 @@ import logging import re import uuid -from datetime import timedelta +from datetime import datetime, timedelta from functools import partial from itertools import product +from typing import cast import pytest +import pytz from appdaemon.types import TimeDeltaLike from appdaemon.utils import parse_timedelta @@ -29,9 +31,12 @@ async def test_run_every( n: int = 2, ) -> None: interval = parse_timedelta(interval) - run_time = (interval * n) + timedelta(seconds=0.01) - register_delay = 0.1 - run_time += timedelta(seconds=register_delay) # Accounts for the delay in registering the callback + + # Calculate base runtime for 'n' occurrences plus a small buffer to account for the delay in registering the callback + register_delay = timedelta(seconds=0.2) + run_time = (interval * (n + 1)) + register_delay + + # If start time is future "now + offset", add offset to ensure coverage if (parts := re.split(r"\s+[\+]\s+", start)) and len(parts) == 2: _, offset = parts run_time += parse_timedelta(offset) @@ -41,11 +46,62 @@ async def test_run_every( app_args = dict(start=start, interval=interval, msg=test_id, register_delay=register_delay) async with run_app_for_time(app_name, run_time=run_time.total_seconds(), **app_args) as (ad, caplog): check_interval_partial = partial(check_interval, caplog, f"kwargs: {{'msg': '{test_id}',") + check_interval_partial(n, interval) - if start.startswith("now -"): - check_interval_partial(n, interval) - else: - check_interval_partial(n + 1, interval) + cb_count = await ad.state.get_state('test', 'admin', f'app.{app_name}', 'instancecallbacks') + assert cast(int, cb_count) >= n, "Callback didn't get called enough times." # diffs = utils.time_diffs(utils.filter_caplog(caplog, test_id)) # logger.debug(diffs) + + +@pytest.mark.asyncio(loop_scope="session") +@pytest.mark.parametrize("start", ["now", "immediate"]) +async def test_run_every_start_time( + run_app_for_time: AsyncTempTest, + start: str, +) -> None: + interval = timedelta(seconds=0.5) + run_time = timedelta(seconds=1) + register_delay = timedelta(seconds=0.1) + + match start: + case "now": + n = 1 + case "immediate": + n = 2 + + app_name = "scheduler_test_app" + test_id = str(uuid.uuid4()) + app_args = dict(start=start, interval=interval, msg=test_id, register_delay=register_delay) + async with run_app_for_time(app_name, run_time=run_time.total_seconds(), **app_args) as (ad, caplog): + check_interval( + caplog, + f"kwargs: {{'msg': '{test_id}',", + n=n, + interval=interval + ) + + cb_count = await ad.state.get_state('test', 'admin', f'app.{app_name}', 'instancecallbacks') + assert cast(int, cb_count) >= (n + 1), "Callback didn't get called enough times." + +now = datetime.now(pytz.utc) +START_TIMES = ["now", now, now.time(), now.isoformat()] + +@pytest.mark.asyncio(loop_scope="session") +@pytest.mark.parametrize("start", START_TIMES) +async def test_run_every_start_time_types( + run_app_for_time: AsyncTempTest, + start: str, +) -> None: + interval = timedelta(seconds=0.25) + run_time = timedelta(seconds=1) + register_delay = timedelta(seconds=0.1) + n = 3 + + app_name = "scheduler_test_app" + test_id = str(uuid.uuid4()) + app_args = dict(start=start, interval=interval, msg=test_id, register_delay=register_delay) + async with run_app_for_time(app_name, run_time=run_time.total_seconds(), **app_args) as (ad, caplog): + cb_count = await ad.state.get_state('test', 'admin', f'app.{app_name}', 'instancecallbacks') + assert cast(int, cb_count) >= (n + 1), "Callback didn't get called enough times."