mirror of
https://github.com/agessaman/meshcore-bot.git
synced 2026-08-23 11:00:10 +00:00
feat(scheduler): airtime guards for {cmd:...}, plus missed docs
Two guards on command placeholders in scheduled messages, neither
configurable, because both exist to protect a shared medium:
A 15-minute floor. A schedule containing {cmd:...} that fires more often
is rejected at startup with an error rather than quietly running slower.
The interval is sampled and measured by the tightest gap between
firings, so "0,1 * * * *" is correctly treated as every 60 seconds and
not as hourly. Schedules without a command placeholder are unaffected.
The command's own cooldown_seconds now applies to a render. Scheduling
is not a way around the rate a command was configured to run at. The
execution is recorded before the command runs, matching execute_commands,
so a slow or failing render cannot be retried straight past the cooldown.
Also fills documentation gaps from the preceding commits:
- --install-extras was only in the script's own --help; documented in
service-installation.md (including alongside --update-venv) and
upgrade.md.
- weather-service.md documented weather_alarm's once-a-day scheduling
with no route to more than one forecast a day, which is exactly what
people go there looking for. It now points at {cmd:wx ...} and notes
that sunrise/sunset still belong to weather_alarm.
This commit is contained in:
+5
-1
@@ -31,7 +31,11 @@ semantic versioning.
|
||||
setting. The command runs for its text only and transmits nothing itself
|
||||
(`CommandManager.render_command_output`); unknown, disabled, admin-only, timing-out
|
||||
and silent commands expand to nothing rather than airing raw placeholder text.
|
||||
Bounded by the new `[Bot] scheduled_command_timeout_seconds` (default 30).
|
||||
Bounded by the new `[Bot] scheduled_command_timeout_seconds` (default 30). Two
|
||||
non-configurable airtime guards apply: a schedule using `{cmd:...}` must not fire
|
||||
more often than every 15 minutes (rejected at startup, measured by the tightest gap
|
||||
so `0,1 * * * *` counts as 60 seconds), and the command's own `cooldown_seconds` is
|
||||
still enforced.
|
||||
- `{path_distance}` is now available in the path command's `[Path_Command] reply_prefix`,
|
||||
reporting total distance travelled (sender → hops → bot, e.g. `12.4km`) and rendering
|
||||
empty when any node in the chain has no usable coordinates. The prefix now supports the
|
||||
|
||||
+8
-1
@@ -547,7 +547,14 @@ category.funfact = fun
|
||||
# disabled, admin-only, times out, or produces no output — the raw {cmd:...} text is
|
||||
# never put on the air. If expansion leaves the message empty, nothing is sent.
|
||||
# Command output is not re-scanned, so a reply containing {cmd:...} cannot recurse.
|
||||
# Mind the airtime: a scheduled command still costs a transmission every time it fires.
|
||||
#
|
||||
# Airtime guards (both deliberate, neither configurable):
|
||||
# - A schedule containing {cmd:...} must not fire more often than every 15 minutes.
|
||||
# An entry that does is rejected at startup with an error and is not scheduled.
|
||||
# Measured by the tightest gap, so "0,1 * * * *" counts as every 60 seconds.
|
||||
# Schedules without a command placeholder are unaffected.
|
||||
# - The command's own cooldown still applies. If it is on cooldown when the schedule
|
||||
# fires, the placeholder expands to nothing that round.
|
||||
#
|
||||
# Available placeholders for mesh network information:
|
||||
#
|
||||
|
||||
@@ -222,12 +222,19 @@ The trigger is matched against command **names and their keywords**, so `{cmd:we
|
||||
|
||||
A placeholder expands to **nothing** (and logs a warning) when the command is unknown, disabled in config, admin-only, times out, or returns no output — the literal `{cmd:...}` is never transmitted. If a message is empty after expansion, nothing is sent at all. Command output is not re-scanned, so a reply that happens to contain `{cmd:...}` cannot recurse.
|
||||
|
||||
Two limits worth knowing:
|
||||
#### Airtime guards
|
||||
|
||||
Every firing is a transmission on a shared medium, and a command placeholder makes it easy to write a cron that airs several times an hour. Two guards apply, and neither is configurable:
|
||||
|
||||
- **A 15-minute floor.** A schedule containing `{cmd:...}` may not fire more often than every 15 minutes. An entry that does is **rejected at startup** with an error and is not scheduled at all — it does not silently run at a slower rate. The check measures the *tightest* gap between firings, so `0,1 * * * *` is treated as a 60-second schedule rather than an hourly one. Schedules with no command placeholder are unaffected.
|
||||
- **The command's own cooldown still applies.** `[<Name>_Command] cooldown_seconds` is not bypassed by scheduling. If the command is on cooldown when the schedule fires, the placeholder expands to nothing for that round and logs a warning.
|
||||
|
||||
Other limits worth knowing:
|
||||
|
||||
- Commands that transmit directly instead of returning text (currently only `announcements`) cannot be rendered and are refused, since running them would broadcast for real.
|
||||
- `[Bot] scheduled_command_timeout_seconds` (default `30`) bounds each render. Network-backed commands like `wx` need the headroom.
|
||||
|
||||
**This costs airtime every time it fires.** A `*/5 * * * *` forecast is 288 transmissions a day; pick an interval the mesh can afford.
|
||||
Even within the floor, mind the cost: a `*/15 * * * *` forecast is 96 transmissions a day.
|
||||
|
||||
### Schedule keys (APScheduler cron, not Vixie)
|
||||
|
||||
|
||||
@@ -58,6 +58,27 @@ attempt to restart that service while preserving the original failure status.
|
||||
|
||||
Read the [upgrade guide](upgrade.md) before upgrading an existing installation.
|
||||
|
||||
### Optional feature packages
|
||||
|
||||
Two feature sets are not installed by default because most bots do not need them:
|
||||
the profanity filter (`better-profanity`, `unidecode`) and geocoding extras
|
||||
(`pycountry`, `us`). A fresh install and a `--upgrade` both prompt for each one.
|
||||
|
||||
For an unattended run, install both without prompting:
|
||||
|
||||
```bash
|
||||
sudo ./install-service.sh --upgrade --install-extras
|
||||
```
|
||||
|
||||
`--install-extras` also works alongside `--update-venv`, which otherwise keeps
|
||||
whatever is already in the virtual environment and does not prompt:
|
||||
|
||||
```bash
|
||||
sudo ./install-service.sh -u --update-venv --install-extras
|
||||
```
|
||||
|
||||
A failed extras install is non-fatal; the bot installs and starts without them.
|
||||
|
||||
## Manual Installation
|
||||
|
||||
If you prefer to install manually:
|
||||
|
||||
@@ -114,6 +114,9 @@ database migration, copies relative-path databases coherently, rewrites the migr
|
||||
configuration, builds a fresh virtual environment, and restarts a service that was
|
||||
active before the upgrade.
|
||||
|
||||
Add `--install-extras` to install the optional profanity-filter and geocoding
|
||||
packages without being prompted, which is what you want for an unattended upgrade.
|
||||
|
||||
Before upgrading, keep a separate backup of your configuration and database. The
|
||||
installer preserves:
|
||||
|
||||
|
||||
@@ -111,6 +111,15 @@ Sends forecast to `weather_channel` at configured time:
|
||||
- Sunrise: `weather_alarm = sunrise`
|
||||
- Sunset: `weather_alarm = sunset`
|
||||
|
||||
`weather_alarm` fires **once a day**. For more than one forecast a day, or a forecast for a location other than the bot's own position, schedule the `wx` command instead with a [`{cmd:...}` placeholder](configuration.md#broadcasting-a-commands-output-cmd):
|
||||
|
||||
```ini
|
||||
[Scheduled_Messages]
|
||||
0 6,12,18 * * * = Public:{cmd:wx Seattle}
|
||||
```
|
||||
|
||||
That accepts any cron schedule and any location, and works the same way for `aqi` and other commands. `sunrise`/`sunset` are not expressible as cron, so keep using `weather_alarm` for those.
|
||||
|
||||
### Rain Nowcast (Proactive)
|
||||
|
||||
Watches your position and posts a heads-up to `rain_channel` (default: `weather_channel`) when precipitation is about to start, using Open-Meteo's 15-minutely forecast — the same engine as the [`rain`/`nowcast` command](command-reference.md#rain-location).
|
||||
|
||||
@@ -1920,6 +1920,19 @@ class CommandManager:
|
||||
)
|
||||
return None
|
||||
|
||||
# The command's own cooldown still governs it. A schedule is not a licence to
|
||||
# run something more often than the operator configured it to run.
|
||||
allowed, remaining = command.check_cooldown()
|
||||
if not allowed:
|
||||
self.logger.warning(
|
||||
"Scheduled {cmd:...} placeholder: %r is on cooldown for another %.0fs; skipped",
|
||||
command_name, remaining,
|
||||
)
|
||||
return None
|
||||
# Recorded before execution, matching execute_commands, so a slow or failing
|
||||
# render cannot be retried straight past the cooldown.
|
||||
command.record_execution()
|
||||
|
||||
sink: list[str] = []
|
||||
synthetic = MeshMessage(
|
||||
content=spec,
|
||||
|
||||
@@ -137,6 +137,18 @@ class MessageScheduler:
|
||||
channel, message, scope = parse_scheduled_message_value(message_info)
|
||||
message = decode_escape_sequences(message)
|
||||
|
||||
if self._has_command_placeholders(message):
|
||||
interval = self._min_fire_interval_seconds(parsed.trigger, tz)
|
||||
floor = self.MIN_COMMAND_PLACEHOLDER_INTERVAL_SECONDS
|
||||
if interval is not None and interval < floor:
|
||||
self.logger.error(
|
||||
"Scheduled message %r uses a {cmd:...} placeholder but fires "
|
||||
"every %.0fs; the minimum is %ds because each firing spends "
|
||||
"airtime. Not scheduled: %s",
|
||||
schedule_key, interval, floor, message,
|
||||
)
|
||||
continue
|
||||
|
||||
job_id = "schedmsg_" + hashlib.sha256(
|
||||
f"{schedule_key}\0{channel}\0{scope or ''}\0{message}".encode()
|
||||
).hexdigest()[:24]
|
||||
@@ -495,6 +507,38 @@ class MessageScheduler:
|
||||
# Non-greedy and brace-free inside, matching the placeholder limits elsewhere.
|
||||
_COMMAND_PLACEHOLDER_RE = re.compile(r"\{cmd:([^{}]+)\}")
|
||||
|
||||
# Floor on how often a schedule containing {cmd:...} may fire. Every firing is a
|
||||
# transmission on a shared medium, and a command placeholder makes it trivial to
|
||||
# write a cron that airs several times an hour. Deliberately not configurable.
|
||||
MIN_COMMAND_PLACEHOLDER_INTERVAL_SECONDS = 900
|
||||
|
||||
@staticmethod
|
||||
def _min_fire_interval_seconds(trigger, tz, samples: int = 12) -> Optional[float]:
|
||||
"""Smallest gap between consecutive firings of *trigger*, in seconds.
|
||||
|
||||
Sampled rather than derived, so uneven crons are measured by their tightest
|
||||
gap: ``0,1 * * * *`` is a 60-second schedule, not a half-hourly one.
|
||||
|
||||
Returns None when the trigger has no future firings to compare.
|
||||
"""
|
||||
now = datetime.datetime.now(tz)
|
||||
previous = trigger.get_next_fire_time(None, now)
|
||||
if previous is None:
|
||||
return None
|
||||
|
||||
smallest = None
|
||||
for _ in range(samples):
|
||||
nxt = trigger.get_next_fire_time(
|
||||
previous, previous + datetime.timedelta(microseconds=1)
|
||||
)
|
||||
if nxt is None:
|
||||
break
|
||||
gap = (nxt - previous).total_seconds()
|
||||
if gap > 0 and (smallest is None or gap < smallest):
|
||||
smallest = gap
|
||||
previous = nxt
|
||||
return smallest
|
||||
|
||||
def _has_command_placeholders(self, message: str) -> bool:
|
||||
return bool(self._COMMAND_PLACEHOLDER_RE.search(message))
|
||||
|
||||
|
||||
@@ -42,6 +42,8 @@ def _make_command(*, name, keywords, reply, enabled=True, admin=False, delay=0.0
|
||||
cmd._derive_config_section_name.return_value = f"{name.title()}_Command"
|
||||
cmd.get_config_value.return_value = enabled
|
||||
cmd.requires_admin_access.return_value = admin
|
||||
# Off cooldown unless a test says otherwise.
|
||||
cmd.check_cooldown.return_value = (True, 0.0)
|
||||
|
||||
async def execute(message):
|
||||
if delay:
|
||||
@@ -270,3 +272,146 @@ class TestRenderWithRealCommand:
|
||||
assert rendered, "a real ping should produce text"
|
||||
mgr.send_dm.assert_not_called()
|
||||
mgr.send_channel_message.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestCommandCooldownStillApplies:
|
||||
"""A schedule is not a licence to outrun a command's configured cooldown."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_render_refused_while_on_cooldown(self, mock_bot):
|
||||
wx = _make_command(name="wx", keywords=["wx"], reply="12C")
|
||||
wx.check_cooldown.return_value = (False, 42.0)
|
||||
mgr = _make_manager(mock_bot, {"wx": wx})
|
||||
assert await mgr.render_command_output("wx Seattle") is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_render_records_execution_so_cooldown_advances(self, mock_bot):
|
||||
wx = _make_command(name="wx", keywords=["wx"], reply="12C")
|
||||
wx.check_cooldown.return_value = (True, 0.0)
|
||||
mgr = _make_manager(mock_bot, {"wx": wx})
|
||||
assert await mgr.render_command_output("wx Seattle") == "12C"
|
||||
wx.record_execution.assert_called_once()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_execution_recorded_even_if_command_then_fails(self, mock_bot):
|
||||
"""Recorded before execute, so a failing render cannot be retried immediately."""
|
||||
boom = _make_command(name="boom", keywords=["boom"], reply=None)
|
||||
boom.check_cooldown.return_value = (True, 0.0)
|
||||
|
||||
async def explode(message):
|
||||
raise RuntimeError("kaboom")
|
||||
|
||||
boom.execute = explode
|
||||
mgr = _make_manager(mock_bot, {"boom": boom})
|
||||
assert await mgr.render_command_output("boom") is None
|
||||
boom.record_execution.assert_called_once()
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestMinimumIntervalFloor:
|
||||
"""{cmd:...} schedules may not fire more often than every 15 minutes."""
|
||||
|
||||
@staticmethod
|
||||
def _interval(cron):
|
||||
import datetime
|
||||
|
||||
from apscheduler.triggers.cron import CronTrigger
|
||||
|
||||
from modules.scheduler import MessageScheduler
|
||||
|
||||
tz = datetime.timezone.utc
|
||||
trigger = CronTrigger.from_crontab(cron, timezone=tz)
|
||||
return MessageScheduler._min_fire_interval_seconds(trigger, tz)
|
||||
|
||||
def test_floor_is_fifteen_minutes(self):
|
||||
from modules.scheduler import MessageScheduler
|
||||
|
||||
assert MessageScheduler.MIN_COMMAND_PLACEHOLDER_INTERVAL_SECONDS == 900
|
||||
|
||||
@pytest.mark.parametrize("cron,expected", [
|
||||
("*/5 * * * *", 300),
|
||||
("*/15 * * * *", 900),
|
||||
("*/30 * * * *", 1800),
|
||||
("0 * * * *", 3600),
|
||||
("0 6,12,18 * * *", 21600),
|
||||
])
|
||||
def test_even_schedules_measured_correctly(self, cron, expected):
|
||||
assert self._interval(cron) == expected
|
||||
|
||||
def test_uneven_cron_measured_by_its_tightest_gap(self):
|
||||
"""0,1 * * * * is a 60-second schedule, not an hourly one."""
|
||||
assert self._interval("0,1 * * * *") == 60
|
||||
|
||||
def test_daily_schedule_is_well_above_the_floor(self):
|
||||
from modules.scheduler import MessageScheduler
|
||||
|
||||
assert self._interval("0 8 * * *") > MessageScheduler.MIN_COMMAND_PLACEHOLDER_INTERVAL_SECONDS
|
||||
|
||||
@pytest.mark.parametrize("cron,allowed", [
|
||||
("* * * * *", False),
|
||||
("*/5 * * * *", False),
|
||||
("*/14 * * * *", False),
|
||||
("0,1 * * * *", False),
|
||||
("*/15 * * * *", True),
|
||||
("*/30 * * * *", True),
|
||||
("0 6,12,18 * * *", True),
|
||||
])
|
||||
def test_floor_admits_and_rejects_the_right_schedules(self, cron, allowed):
|
||||
from modules.scheduler import MessageScheduler
|
||||
|
||||
interval = self._interval(cron)
|
||||
floor = MessageScheduler.MIN_COMMAND_PLACEHOLDER_INTERVAL_SECONDS
|
||||
assert (interval >= floor) is allowed
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestFloorEnforcedDuringSetup:
|
||||
"""The floor has to actually stop the job being scheduled, not just compute a number."""
|
||||
|
||||
@staticmethod
|
||||
def _run_setup(entries):
|
||||
import configparser
|
||||
from unittest.mock import patch
|
||||
|
||||
from modules.scheduler import MessageScheduler
|
||||
|
||||
config = configparser.ConfigParser()
|
||||
config["Bot"] = {"timezone": "UTC"}
|
||||
config["Scheduled_Messages"] = entries
|
||||
|
||||
bot = MagicMock()
|
||||
bot.config = config
|
||||
|
||||
sched = object.__new__(MessageScheduler)
|
||||
sched.bot = bot
|
||||
sched.logger = MagicMock()
|
||||
sched.scheduled_messages = {}
|
||||
sched._shutdown_apscheduler_if_running = MagicMock()
|
||||
sched._setup_device_mode_scheduler_jobs = MagicMock()
|
||||
sched.setup_interval_advertising = MagicMock()
|
||||
|
||||
fake_scheduler = MagicMock()
|
||||
with patch("modules.scheduler.BackgroundScheduler", return_value=fake_scheduler):
|
||||
sched.setup_scheduled_messages()
|
||||
return sched, fake_scheduler
|
||||
|
||||
def test_too_frequent_command_schedule_is_not_added(self):
|
||||
sched, apsched = self._run_setup({"*/5 * * * *": "Public:{cmd:wx Seattle}"})
|
||||
assert apsched.add_job.call_count == 0
|
||||
assert sched.scheduled_messages == {}
|
||||
assert sched.logger.error.called
|
||||
|
||||
def test_uneven_cron_with_tight_gap_is_not_added(self):
|
||||
_, apsched = self._run_setup({"0,1 * * * *": "Public:{cmd:wx Seattle}"})
|
||||
assert apsched.add_job.call_count == 0
|
||||
|
||||
def test_schedule_at_the_floor_is_added(self):
|
||||
sched, apsched = self._run_setup({"*/15 * * * *": "Public:{cmd:wx Seattle}"})
|
||||
assert apsched.add_job.call_count == 1
|
||||
assert sched.scheduled_messages
|
||||
|
||||
def test_frequent_schedule_without_placeholder_is_unaffected(self):
|
||||
"""The floor applies to command placeholders, not to plain scheduled text."""
|
||||
_, apsched = self._run_setup({"*/5 * * * *": "Public:static message"})
|
||||
assert apsched.add_job.call_count == 1
|
||||
|
||||
Reference in New Issue
Block a user