From 6e8d543d2a2cfa6a2de8fa49411964fd57c2e808 Mon Sep 17 00:00:00 2001 From: maziggy Date: Thu, 24 Sep 2026 11:59:30 +0200 Subject: [PATCH] fix(ams): stop showing the humidity drop index as a percentage (issue #3140) Bambu sends two humidity fields that are not the same quantity. humidity_raw is relative humidity in percent; humidity is a 1-5 drop index, and it runs the other way -- OpenBambuAPI's push_info sample pairs "humidity:30%" with "humidity_idx:4", so a high index means dry where a high percentage means wet. Four call sites used the index whenever no percentage arrived. A unit sending only the index therefore rendered as "2%" in the green band while being the second-wettest of the five steps, charted an average of index values as a percentage, and sat under every humidity threshold forever, since no index can reach one -- the alarm and auto-drying could not fire for such a unit at all. - utils/ams_humidity: one leaf helper, a percentage or None. The index is never converted; None is what every caller already handles. - routes/printers, printer_manager, print_scheduler, main, bambu_mqtt: all five readings go through it, so the card, the websocket, the chart, the alarm and auto-drying cannot answer differently. - main: a unit that reports the index and no usable percentage says so once per unit in the log, with its firmware versions requested. No supported printer is known to do this, and "known" is doing work there -- the alternative is a card that goes blank with no trace. Three faults found while checking what else those paths touched: - main: humidity_raw=float(x) if x else None stored NULL for a numeric 0% while writing 0.0 to humidity on the same row. - main: that same expression was unguarded, unlike the parse above it, so a non-numeric humidity_raw raised inside record_ams_history and aborted the pass for every printer, not just the one that sent it. - routes/ams_history: the averages were tested for truthiness, so a window averaging exactly 0 reported no average while the min and max beside it reported 0.0. An affected unit now reports no humidity rather than a number that means the opposite: the indicator is hidden, the chart leaves a gap, the alarm and auto-drying skip the unit. Temperature is untouched. Auto-drying's outcome is unchanged either way -- an index could never cross the threshold -- so only the intent moves. No supported printer is known to be affected; the report came from an install running X1Plus, which Bambuddy does not support. Verified against 7927 recorded samples from seven AMS units including an AMS-HT: not one used the fallback. Two percentages that did fall through to the index no longer do -- a reading with a decimal point, and "38.0", which int() rejected. --- CHANGELOG.md | 1 + backend/app/api/routes/ams_history.py | 8 +- backend/app/api/routes/printers.py | 22 +- backend/app/main.py | 51 ++- backend/app/services/bambu_mqtt.py | 3 +- backend/app/services/print_scheduler.py | 22 +- backend/app/services/printer_manager.py | 21 +- backend/app/utils/ams_humidity.py | 40 +++ .../tests/integration/test_ams_history_api.py | 38 +++ .../test_ams_humidity_index_guard_3140.py | 312 ++++++++++++++++++ 10 files changed, 453 insertions(+), 65 deletions(-) create mode 100644 backend/app/utils/ams_humidity.py create mode 100644 backend/tests/unit/test_ams_humidity_index_guard_3140.py diff --git a/CHANGELOG.md b/CHANGELOG.md index dbbde58ba..74221ff7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ All notable changes to Bambuddy will be documented in this file. - **Every FTP session Bambuddy opens now records how it closed (#3009, reported by @grengojbo)** — the report traced a print completion that opened two FTP connections to the printer, deleted one file and then, as far as the log showed, did nothing else until the printer was powered off 21 minutes later, and concluded the connections were being left open. They were not: the post-print SD-card cleanup opens one connection per candidate filename and closes each in a `finally`, which a run against a real FTPS server confirms at the server end for both the delete and the 550 not-here case. The trouble is that nothing in the log could have said so. Neither the clean close nor the hard socket drop logged anything at any level, so a session closed properly and a socket genuinely abandoned produced the same output — none — and the only way to tell them apart was to read the source. Both now log one DEBUG line naming the printer, whether QUIT was acknowledged or the socket had to be dropped without it, why, and how long the session was held. Every connect in a debug log is now paired with a close, so the next person suspecting a leaked FTP connection can settle it from a support bundle rather than by inference. Nothing about the connection handling itself changed, and at default log level nothing new is printed. This does not explain the SD-card read/write error in that report or in #645; it only removes one theory from the list by making it checkable. ### Fixed +- **An AMS that reports no humidity percentage no longer shows the drop index as one (#3140, reported by @Sawtaytoes)** — Bambu sends two humidity fields that are not the same quantity: `humidity_raw` is relative humidity in percent, and `humidity` is the 1-5 drop index, which runs the other way — a high index means dry where a high percentage means wet. Bambuddy used the index whenever no percentage arrived, so a unit sending only the index rendered as "2%" in the green band while being the second-wettest of the five steps, charted an average of index values as a percentage, and could never cross a humidity alarm or auto-drying threshold, since no index reaches one. Such a unit now reports no humidity at all: the card hides the water-drop indicator, the history chart leaves a gap, and the alarm and auto-drying skip the unit instead of reading it as permanently bone dry. Temperature is recorded and alarmed on as before. No supported printer is known to be affected — the report came from an install running X1Plus, which Bambuddy does not support — and printers that send a percentage keep it, including two readings that previously fell through to the index because they would not parse as a whole number: one with a decimal point, and `38.0`. A unit that sends the index and no percentage now says so once in the log, with its firmware versions requested, so a supported printer that turns out to do this is visible rather than silently blank. Three smaller faults in the same paths went with it: a humidity of exactly 0% was stored as NULL when the firmware sent it as a number, a history window averaging exactly 0 reported no average at all while the minimum and maximum beside it reported 0, and a `humidity_raw` that was not a number at all aborted the whole recording pass for every printer, not just the one that sent it. - **Two Bambuddy instances could not have their CA certificates trusted at the same time (#3014, reported by @Steven-Pierce)** — a slicer holding the CA of two Bambuddy installs failed to connect to one of them, with the same generic `code=-1` an install whose CA was never imported gives. Each CA worked on its own; together, one stopped. Every install signed its virtual printers with an authority named exactly `CN=Virtual Printer CA`, and a slicer's trust store is a flat list of certificates that OpenSSL resolves by subject name: it takes the first authority whose name matches the certificate in front of it and fails the chain when that authority turns out not to have signed it, rather than trying the next match. Whichever of the two landed second in the file therefore lost, decided by nothing more than the order they were appended in. A newly generated CA now carries a per-install suffix taken from its own key — `CN=Virtual Printer CA D55808BE` — and publishes that key identifier, which the printer certificate then points back at, so the two are told apart by name and each chain resolves to the authority that really signed it. Nothing has to be re-imported on upgrade: an install that already has a CA keeps it untouched, and the printer certificates signed by it keep exactly the shape they have today. The collision goes away as soon as one of the two CAs is newer than this release — a second install set up after upgrading, or an existing one whose `bbl_ca.crt` and `bbl_ca.key` are deleted from `virtual_printer/certs/` so a fresh CA is generated, which does mean importing that one into the slicer again. - **ntfy per-event priorities were set, stored, and never sent (#3139, reported by @Thomansky)** — a per-event priority (#990) never reached ntfy: every notification arrived at the server's default, whatever the dialog showed. The two halves of the feature disagreed about the key. The dialog builds its rows from the provider's event toggles and stores the map under those names — `on_print_complete`, `on_print_failed` — while every sender is called with the bare event name, `print_complete`, so the lookup missed for all 18 events the dialog offers. It looked healthy from both ends: the stored config held exactly the priorities that were set, and the tests were green because they called the ntfy sender directly with the prefixed name, the one spelling the running system never produces. The lookup now accepts either spelling, so existing configurations keep working and nothing has to be migrated. The tests use the bare form, and one of them now runs from a finished print through to the outgoing request, which is the assertion that would have caught this. On upgrade, priorities that have been sitting in the database inertly start being honoured, so an event mapped to **Min** or **Low** will now arrive quieter than it did yesterday — that is the setting taking effect, not alerts going missing. The daily digest is unchanged and still goes out at the server default: it summarises several events at once, and the dialog offers no priority for it. - **Restoring a backup into a different version of Bambuddy failed, and the failure took the live database with it** — A backup carries the schema of the install that made it, and the PostgreSQL restore path does not use that schema: it drops every table, recreates them from the running process's ORM, and inserts the backup's columns into the result. Any NOT NULL column the running version has and the backup does not then arrives with nothing to put in it, and a model-side `default=` cannot help, because SQLAlchemy applies one to ORM and Core inserts and never to the raw `text()` SQL this import builds — `create_all` emits no DDL default for it either. A backup made after `user_wallets.currency` was dropped in #3123, restored onto an install that still had the column, died on `null value in column "currency"` with the drop already committed: the restore's first transaction wipes the database, so the install was left with an empty schema, the previous data gone, and the MFA key file already replaced by the backup's — whatever had survived would have been encrypted under a key that no longer matched. The import now fills such a column from the model's default (evaluating a callable one), leaves a column with a server default to the database rather than second-guessing a timestamp it is better placed to produce, and ignores a nullable one exactly as before. A column that genuinely cannot be filled is refused **before anything is touched** — before services are paused, before the key file is written, before the first `DROP` — and the same applies to a `bambuddy.db` that is not a readable SQLite database at all, which previously was discovered only by the import that had already dropped the tables. Backups now carry a `manifest.json` naming the version that wrote them, so the refusal reads "this backup was made by X, this install runs Y" rather than a list of column names; a backup taken before the manifest existed restores exactly as it did, with the version simply unknown. The refusal is an HTTP 400, and the frontend was dropping its body on the floor — a refused restore is an `HTTPException`, so its `{detail}` is not the `{success, message}` a successful one returns, and the modal raised an empty error toast — so the reason now reaches the operator who has to act on it. A SQLite install is unaffected throughout: it restores by copying the backup's pages, schema included, and migrates forward afterwards. diff --git a/backend/app/api/routes/ams_history.py b/backend/app/api/routes/ams_history.py index a781a7dd8..d4bb0fc13 100644 --- a/backend/app/api/routes/ams_history.py +++ b/backend/app/api/routes/ams_history.py @@ -93,10 +93,14 @@ async def get_ams_history( ], min_humidity=stats.min_humidity, max_humidity=stats.max_humidity, - avg_humidity=round(stats.avg_humidity, 1) if stats.avg_humidity else None, + # ``is not None``, not truthiness: an average of exactly 0 is a + # reading, and the min/max beside it would report it while the average + # showed an em dash. AVG over an empty or all-NULL window is the only + # case that has no answer (#3140). + avg_humidity=round(stats.avg_humidity, 1) if stats.avg_humidity is not None else None, min_temperature=stats.min_temp, max_temperature=stats.max_temp, - avg_temperature=round(stats.avg_temp, 1) if stats.avg_temp else None, + avg_temperature=round(stats.avg_temp, 1) if stats.avg_temp is not None else None, ) diff --git a/backend/app/api/routes/printers.py b/backend/app/api/routes/printers.py index f6ad99e54..56bad42f4 100644 --- a/backend/app/api/routes/printers.py +++ b/backend/app/api/routes/printers.py @@ -90,6 +90,7 @@ from backend.app.services.printer_media import ( ) from backend.app.services.slicer_filament_resolver import _ORCA_PROFILE_ID from backend.app.services.slot_nozzle import resolve_slot_nozzle +from backend.app.utils.ams_humidity import ams_humidity_percent from backend.app.utils.filament_ids import filament_id_to_setting_id from backend.app.utils.filament_types import is_material_name, printer_filament_type from backend.app.utils.fts_routing import slot_extruder @@ -586,22 +587,11 @@ async def get_printer_status( exists=tray_data.get("exists"), ) ) - # Prefer humidity_raw (percentage) over humidity (index 1-5) - # humidity_raw is the actual percentage value from the sensor - humidity_raw = ams_data.get("humidity_raw") - humidity_idx = ams_data.get("humidity") - humidity_value = None - - if humidity_raw is not None: - try: - humidity_value = int(humidity_raw) - except (ValueError, TypeError): - pass # Skip unparseable humidity; will try index fallback - if humidity_value is None and humidity_idx is not None: - try: - humidity_value = int(humidity_idx) - except (ValueError, TypeError): - pass # Skip unparseable humidity index; humidity remains None + # Percentage only. The 1-5 ``humidity`` index is never substituted + # for one -- it is inverted, so it would read as the opposite of + # what it means (#3140). See utils/ams_humidity. + humidity_pct = ams_humidity_percent(ams_data) + humidity_value = int(round(humidity_pct)) if humidity_pct is not None else None # AMS-HT has 1 tray, regular AMS has 4 trays is_ams_ht = len(trays) == 1 diff --git a/backend/app/main.py b/backend/app/main.py index 50836afa6..1833c1433 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -147,6 +147,7 @@ from backend.app.services.spoolman_tracking import ( ) from backend.app.services.tasmota import tasmota_service from backend.app.utils.ams_drying import is_drying_active, temperature_alarm_suppressed +from backend.app.utils.ams_humidity import ams_humidity_percent from backend.app.utils.filament_types import printer_filament_type from backend.app.utils.fts_routing import extruder_for_inlet from backend.app.utils.local_time import utcnow_naive @@ -7846,6 +7847,11 @@ _ams_cleanup_counter = 0 # Track recordings to trigger periodic cleanup # Track alarm cooldowns (printer_id:ams_id:type -> last_alarm_time) _ams_alarm_cooldown: dict[str, datetime] = {} AMS_ALARM_COOLDOWN_MINUTES = 60 # Don't send same alarm more than once per hour +# (printer_id, ams_id) already reported as sending the drop index and no +# percentage. Logged once each so a supported printer that turns out to do this +# shows up in a support bundle rather than as a user wondering where the +# humidity reading went -- see the note at the read site below (#3140). +_ams_index_only_logged: set[tuple[int, int]] = set() def _resolve_temp_alarm_threshold(fair_threshold: float, raw_alarm_value: str | None) -> float: @@ -8083,20 +8089,30 @@ async def record_ams_history(): for ams_data in raw_data["ams"]: ams_id = int(ams_data.get("id", 0)) - # Get humidity (prefer humidity_raw) - humidity_raw = ams_data.get("humidity_raw") - humidity_idx = ams_data.get("humidity") - humidity = None - if humidity_raw is not None: - try: - humidity = float(humidity_raw) - except (ValueError, TypeError): - pass # Skip unparseable humidity; will try fallback - if humidity is None and humidity_idx is not None: - try: - humidity = float(humidity_idx) - except (ValueError, TypeError): - pass # Skip unparseable humidity index value + # Percentage only. The 1-5 index is inverted, so + # charting it as a percentage drew the wettest units as + # the driest (#3140); a unit that reports no percentage + # leaves a gap in the chart instead. See + # utils/ams_humidity. + humidity = ams_humidity_percent(ams_data) + + # No supported printer is known to send the index + # alone -- the report came from unsupported firmware, + # and no install has been seen using the old fallback. + # "Known" is doing work there, so say so once per unit: + # the alternative is a silent blank card. + if humidity is None and ams_data.get("humidity") is not None: + unit_key = (printer.id, ams_id) + if unit_key not in _ams_index_only_logged: + _ams_index_only_logged.add(unit_key) + logger.info( + "[%s] AMS %d reports the 1-5 humidity index but no usable humidity_raw " + "percentage. The index is inverted and is not shown as a percentage " + "(#3140), so this unit has no humidity reading, chart or alarm. " + "Please report this with the printer and AMS firmware versions.", + printer.name, + ams_id, + ) # Get temperature temperature = None @@ -8116,7 +8132,12 @@ async def record_ams_history(): printer_id=printer.id, ams_id=ams_id, humidity=humidity, - humidity_raw=float(humidity_raw) if humidity_raw else None, + # Both columns hold the same reading now that the + # index can no longer reach ``humidity``. Writing it + # through the same value also stops a genuine 0% + # from being stored as NULL, which the old truthiness + # test did. + humidity_raw=humidity, temperature=temperature, ) db.add(history) diff --git a/backend/app/services/bambu_mqtt.py b/backend/app/services/bambu_mqtt.py index a0fd0e9ca..3da2b9ba3 100644 --- a/backend/app/services/bambu_mqtt.py +++ b/backend/app/services/bambu_mqtt.py @@ -24,6 +24,7 @@ import paho.mqtt.client as mqtt from backend.app.services.hms_actions import HMSAction, get_actions_for_error_code from backend.app.services.hms_errors import describe_fault from backend.app.utils.ams_drying import ACTIVE_DRY_STATUSES +from backend.app.utils.ams_humidity import ams_humidity_percent from backend.app.utils.paho_teardown import retire_paho_client logger = logging.getLogger(__name__) @@ -3696,7 +3697,7 @@ class BambuMQTTClient: cycle ending at 63 degC with the reading still above the threshold is the whole shape of the re-arm loop. """ - box = f"temp={ams_unit.get('temp')} humidity={ams_unit.get('humidity_raw', ams_unit.get('humidity'))}" + box = f"temp={ams_unit.get('temp')} humidity={ams_humidity_percent(ams_unit)}" if ams_id in self._drying_stops_sent: self._drying_stops_sent.discard(ams_id) logger.info( diff --git a/backend/app/services/print_scheduler.py b/backend/app/services/print_scheduler.py index 50aae6396..0680644ff 100644 --- a/backend/app/services/print_scheduler.py +++ b/backend/app/services/print_scheduler.py @@ -59,6 +59,7 @@ from backend.app.services.printer_manager import ( supports_drying_while_printing, ) from backend.app.services.smart_plug_manager import smart_plug_manager +from backend.app.utils.ams_humidity import ams_humidity_percent from backend.app.utils.color_utils import perceptual_color_distance from backend.app.utils.filament_types import canonical_filament_type from backend.app.utils.filename import derive_remote_filename @@ -4297,21 +4298,12 @@ class PrintScheduler: dry_time = int(ams_data.get("dry_time") or 0) - # Read humidity — prefer humidity_raw (actual %) over humidity (index 1-5) - humidity = None - h_raw = ams_data.get("humidity_raw") - if h_raw is not None: - try: - humidity = int(h_raw) - except (ValueError, TypeError): - pass - if humidity is None: - h_idx = ams_data.get("humidity") - if h_idx is not None: - try: - humidity = int(h_idx) - except (ValueError, TypeError): - pass + # Read humidity as a percentage. The 1-5 index is never + # substituted: it is inverted, and being unable to exceed any + # threshold it would read as "dry" forever (#3140). ``None`` + # already means "skip this unit" everywhere below. + humidity_pct = ams_humidity_percent(ams_data) + humidity = int(round(humidity_pct)) if humidity_pct is not None else None unit_key = (pid, ams_id) unit_state = self._auto_dry_units.get(unit_key) diff --git a/backend/app/services/printer_manager.py b/backend/app/services/printer_manager.py index 66a535038..08c6e6688 100644 --- a/backend/app/services/printer_manager.py +++ b/backend/app/services/printer_manager.py @@ -15,6 +15,7 @@ from backend.app.services.bambu_mqtt import ( PrinterState, get_stage_name, ) +from backend.app.utils.ams_humidity import ams_humidity_percent from backend.app.utils.kprofile_lookup import build_slot_k_resolver logger = logging.getLogger(__name__) @@ -1399,22 +1400,10 @@ def printer_state_to_dict( "exists": tray.get("exists"), } ) - # Prefer humidity_raw (actual percentage) over humidity (index 1-5) - humidity_raw = ams_data.get("humidity_raw") - humidity_idx = ams_data.get("humidity") - humidity_value = None - - if humidity_raw is not None: - try: - humidity_value = int(humidity_raw) - except (ValueError, TypeError): - pass # Skip unparseable humidity; will try index fallback - # Fall back to index if no raw value (index is 1-5, not percentage) - if humidity_value is None and humidity_idx is not None: - try: - humidity_value = int(humidity_idx) - except (ValueError, TypeError): - pass # Skip unparseable humidity index; humidity remains None + # Percentage only — the 1-5 index is inverted and must never stand + # in for one (#3140). See utils/ams_humidity. + humidity_pct = ams_humidity_percent(ams_data) + humidity_value = int(round(humidity_pct)) if humidity_pct is not None else None # AMS-HT has 1 tray, regular AMS has 4 trays is_ams_ht = len(trays) == 1 diff --git a/backend/app/utils/ams_humidity.py b/backend/app/utils/ams_humidity.py new file mode 100644 index 000000000..89ee46068 --- /dev/null +++ b/backend/app/utils/ams_humidity.py @@ -0,0 +1,40 @@ +"""Shared reading of an AMS unit's humidity. + +Bambu sends two humidity fields and they are not the same quantity. +``humidity_raw`` is relative humidity in percent. ``humidity`` is a 1-5 drop +index, and it runs the other way: OpenBambuAPI's push_info sample carries both +in one line -- ``ams0 temp:18.4;humidity:30%;humidity_idx:4`` -- so a high index +means dry where a high percentage means wet. + +Falling back from one to the other therefore does not degrade, it inverts. +Index 2 rendered as "2%" reads as the driest a unit can be while the unit is in +fact the second-wettest of the five steps, and no index can ever exceed a +percentage threshold, so the humidity alarm and auto-drying silently never fire +for such a unit (#3140). A unit that reports no percentage has no percentage: +this returns ``None``, which every caller already treats as "no reading" -- the +card hides the indicator, the alarm and auto-drying skip the unit, and the +history chart leaves a gap. + +Kept as a leaf module like ``ams_drying``: nothing here imports from the app. +""" + +from collections.abc import Mapping +from typing import Any + + +def ams_humidity_percent(ams_data: Any) -> float | None: + """Relative humidity in percent for one AMS unit, or ``None``. + + ``None`` covers every case where the unit did not report a usable + percentage, including the units that send only the 1-5 index -- which is + deliberately never converted. See the module docstring. + """ + if not isinstance(ams_data, Mapping): + return None + raw = ams_data.get("humidity_raw") + if raw is None: + return None + try: + return float(raw) + except (TypeError, ValueError): + return None # Unparseable reading — not a licence to use the index diff --git a/backend/tests/integration/test_ams_history_api.py b/backend/tests/integration/test_ams_history_api.py index f90c09f75..f0089be9a 100644 --- a/backend/tests/integration/test_ams_history_api.py +++ b/backend/tests/integration/test_ams_history_api.py @@ -85,6 +85,44 @@ class TestAMSHistoryAPI: assert data["min_temperature"] == 24.0 assert data["max_temperature"] == 26.0 + @pytest.mark.asyncio + @pytest.mark.integration + async def test_an_average_of_zero_is_reported_as_zero( + self, async_client: AsyncClient, ams_history_factory, printer_factory, db_session + ): + """A window whose readings are all 0 has an average of 0, not "no data". + + The response used to test the average for truthiness, so min and max + reported 0.0 while the average beside them came back null and the card + showed an em dash (#3140). Zero is rare but real -- a warm unit part way + through a drying cycle reaches it. + """ + printer = await printer_factory() + await ams_history_factory(printer_id=printer.id, humidity=0.0, temperature=0.0) + await ams_history_factory(printer_id=printer.id, humidity=0.0, temperature=0.0) + + response = await async_client.get(f"/api/v1/ams-history/{printer.id}/0") + assert response.status_code == 200 + data = response.json() + + assert data["min_humidity"] == 0.0 + assert data["avg_humidity"] == 0.0 + assert data["avg_temperature"] == 0.0 + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_an_empty_window_still_has_no_average(self, async_client: AsyncClient, printer_factory): + """The one case that genuinely has no answer must stay null.""" + printer = await printer_factory() + + response = await async_client.get(f"/api/v1/ams-history/{printer.id}/0") + assert response.status_code == 200 + data = response.json() + + assert data["data"] == [] + assert data["avg_humidity"] is None + assert data["avg_temperature"] is None + @pytest.mark.asyncio @pytest.mark.integration async def test_get_ams_history_with_hours_filter( diff --git a/backend/tests/unit/test_ams_humidity_index_guard_3140.py b/backend/tests/unit/test_ams_humidity_index_guard_3140.py new file mode 100644 index 000000000..82441c8a6 --- /dev/null +++ b/backend/tests/unit/test_ams_humidity_index_guard_3140.py @@ -0,0 +1,312 @@ +"""The 1-5 humidity index is never shown or stored as a percentage (#3140). + +Bambu sends ``humidity_raw`` (relative humidity, percent) and ``humidity`` (a +1-5 drop index). The index runs the other way -- OpenBambuAPI's push_info +sample pairs ``humidity:30%`` with ``humidity_idx:4`` -- so substituting one +for the other inverts the reading rather than approximating it. A unit sending +only the index used to render as "2%" in the good/green band while being the +second-wettest of the five steps, chart an average of index values as a +percentage, and sit under every alarm threshold forever. + +The reporting install ran X1Plus, which Bambuddy does not support, and no +stock-firmware printer is on record as sending the index alone. The guard is +kept regardless because it is about what we do when the field is missing for +any reason, and showing a number we cannot interpret is worse than showing +none: every consumer of these serializers already handles ``None`` by hiding +the indicator, skipping the unit or leaving a gap in the chart. +""" + +import asyncio +import logging +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + +import backend.app.main as main +from backend.app.api.routes.printers import get_printer_status +from backend.app.models.ams_history import AMSSensorHistory +from backend.app.models.printer import Printer +from backend.app.services.bambu_mqtt import PrinterState +from backend.app.services.printer_manager import printer_state_to_dict +from backend.app.utils.ams_humidity import ams_humidity_percent + +# --- the reading itself --- + + +def test_a_reported_percentage_is_the_reading(): + assert ams_humidity_percent({"humidity_raw": 45}) == 45.0 + assert ams_humidity_percent({"humidity_raw": "45"}) == 45.0 + + +def test_a_fractional_percentage_survives(): + """``int("16.5")`` raised, which sent the old code to the index fallback -- + so a sensor reporting one decimal place read as a drop index.""" + assert ams_humidity_percent({"humidity_raw": "16.5"}) == 16.5 + + +def test_the_index_alone_is_not_a_reading(): + """The guard. Index 2 is the second-wettest step, and 2% is as dry as a + unit can read -- the two are not interchangeable in either direction.""" + assert ams_humidity_percent({"humidity": 2}) is None + assert ams_humidity_percent({"humidity": "2"}) is None + + +def test_the_percentage_wins_when_both_are_present(): + assert ams_humidity_percent({"humidity": 4, "humidity_raw": "62"}) == 62.0 + + +def test_a_genuine_zero_is_a_reading(): + """Not ``None``: the history writer used to test truthiness, so a unit + reading 0% stored NULL while the same pass wrote 0.0 to the other column.""" + assert ams_humidity_percent({"humidity_raw": 0}) == 0.0 + assert ams_humidity_percent({"humidity_raw": "0"}) == 0.0 + + +def test_an_unparseable_percentage_is_not_a_licence_to_use_the_index(): + assert ams_humidity_percent({"humidity_raw": "n/a", "humidity": 3}) is None + assert ams_humidity_percent({"humidity_raw": None, "humidity": 3}) is None + + +def test_a_unit_that_is_not_a_mapping_reads_as_no_unit(): + assert ams_humidity_percent(None) is None + assert ams_humidity_percent("ams0") is None + + +# --- what the two serializers of the same card report --- + + +def _index_only_unit() -> dict: + return { + "ams": [ + { + "id": 0, + "humidity": "2", # index, no humidity_raw + "temp": "24.0", + "tray": [{"id": 0, "tray_type": "PLA"}], + } + ] + } + + +def test_the_websocket_serializer_reports_no_humidity_for_an_index_only_unit(): + result = printer_state_to_dict(PrinterState(connected=True, state="IDLE", raw_data=_index_only_unit())) + + assert result["ams"][0]["humidity"] is None + assert result["ams"][0]["temp"] == "24.0" # the other sensor is unaffected + + +@pytest.mark.asyncio +async def test_the_rest_serializer_reports_no_humidity_for_an_index_only_unit(db_session): + """The two serializers feed the same card and must not answer differently.""" + printer = Printer(name="X1C", serial_number="S-3140", ip_address="1.1.1.1", access_code="c", model="X1C") + db_session.add(printer) + await db_session.commit() + + state = PrinterState(connected=True, state="IDLE", raw_data=_index_only_unit()) + + with patch("backend.app.api.routes.printers.printer_manager") as mock_pm: + mock_pm.get_status.return_value = state + mock_pm.get_drying_targets.return_value = {} + status = await get_printer_status(printer.id, db=db_session) + + assert status.ams[0].humidity is None + + +@pytest.mark.asyncio +async def test_the_rest_serializer_still_reports_a_percentage(db_session): + """The guard must not cost the supported case its reading.""" + printer = Printer(name="H2D", serial_number="S-3140b", ip_address="1.1.1.2", access_code="c", model="H2D") + db_session.add(printer) + await db_session.commit() + + raw = _index_only_unit() + raw["ams"][0]["humidity_raw"] = "38" + state = PrinterState(connected=True, state="IDLE", raw_data=raw) + + with patch("backend.app.api.routes.printers.printer_manager") as mock_pm: + mock_pm.get_status.return_value = state + mock_pm.get_drying_targets.return_value = {} + status = await get_printer_status(printer.id, db=db_session) + + assert status.ams[0].humidity == 38 + + +# --- what the recorder writes, and what it alarms on --- + + +def _state_with(unit: dict) -> PrinterState: + return PrinterState(connected=True, state="IDLE", raw_data={"ams": [unit]}) + + +async def _run_one_pass(test_engine, unit: dict): + """One pass of record_ams_history against a single AMS unit. + + Same shape as test_ams_temp_alarm_dispatch_2905: the loop is a no-arg + infinite task, so a fake sleep that recognises its own intervals runs + exactly one pass and then cancels it. + """ + real_sleep = asyncio.sleep + + async def fake_sleep(seconds): + if seconds == 10: # startup wait before the first pass + return + if seconds == main.AMS_HISTORY_INTERVAL: # pass finished cleanly + raise asyncio.CancelledError + if seconds == 60: # the loop's own except-Exception backoff + raise AssertionError("record_ams_history raised; check the warning log") + await real_sleep(seconds) + + service = MagicMock() + service.on_ams_humidity_high = AsyncMock() + service.on_ams_ht_humidity_high = AsyncMock() + service.on_ams_temperature_high = AsyncMock() + service.on_ams_ht_temperature_high = AsyncMock() + + maker = async_sessionmaker(test_engine, class_=AsyncSession, expire_on_commit=False) + cooldown_before = dict(main._ams_alarm_cooldown) + counter_before = main._ams_cleanup_counter + main._ams_alarm_cooldown.clear() + try: + with ( + patch.object(main, "async_session", maker), + patch.object(main, "notification_service", service), + patch.object(main.printer_manager, "get_status", return_value=_state_with(unit)), + patch.object(main.asyncio, "sleep", fake_sleep), + ): + await main.record_ams_history() + finally: + main._ams_alarm_cooldown.clear() + main._ams_alarm_cooldown.update(cooldown_before) + main._ams_cleanup_counter = counter_before + return service + + +async def _rows(db_session, printer_id: int) -> list[AMSSensorHistory]: + result = await db_session.execute(select(AMSSensorHistory).where(AMSSensorHistory.printer_id == printer_id)) + return list(result.scalars().all()) + + +async def _printer(db_session, serial: str) -> Printer: + printer = Printer(name="X1C", serial_number=serial, ip_address="1.1.1.1", access_code="c", model="X1C") + db_session.add(printer) + await db_session.commit() + return printer + + +@pytest.mark.asyncio +async def test_an_index_only_unit_charts_a_gap_not_a_percentage(db_session, test_engine): + """The temperature is still worth recording, so the row is written -- but + with no humidity, which the chart draws as a gap rather than as a flat 2% + line in the good band.""" + printer = await _printer(db_session, "S-3140c") + + service = await _run_one_pass( + test_engine, + {"id": 0, "humidity": "2", "temp": "24.0", "tray_exist_bits": "1", "tray": [{"tray_type": "PLA"}]}, + ) + + rows = await _rows(db_session, printer.id) + assert len(rows) == 1 + assert rows[0].humidity is None + assert rows[0].humidity_raw is None + assert rows[0].temperature == 24.0 + assert service.on_ams_humidity_high.await_count == 0 + + +@pytest.mark.asyncio +async def test_an_index_only_unit_says_so_in_the_log_once(db_session, test_engine, caplog): + """A blank humidity field on a supported printer would otherwise be silent. + + Nothing on record says a supported printer sends the index alone, and the + guard makes such a unit stop reporting humidity entirely -- so it names + itself in the log, once per unit, rather than leaving the card blank with + no explanation anywhere. + """ + printer = await _printer(db_session, "S-3140f") + main._ams_index_only_logged.clear() + unit = {"id": 0, "humidity": "2", "temp": "24.0", "tray_exist_bits": "1", "tray": [{"tray_type": "PLA"}]} + + try: + with caplog.at_level(logging.INFO, logger="backend.app.main"): + await _run_one_pass(test_engine, unit) + await _run_one_pass(test_engine, unit) + finally: + main._ams_index_only_logged.clear() + + lines = [r.getMessage() for r in caplog.records if "humidity index" in r.getMessage()] + assert len(lines) == 1 + assert printer.name in lines[0] + assert "#3140" in lines[0] + + +@pytest.mark.asyncio +async def test_a_unit_reporting_no_humidity_at_all_is_not_logged(db_session, test_engine, caplog): + """The note is about a unit whose index we are declining to use. A unit + that sends neither field is not new and has nothing to report.""" + await _printer(db_session, "S-3140g") + main._ams_index_only_logged.clear() + + try: + with caplog.at_level(logging.INFO, logger="backend.app.main"): + await _run_one_pass( + test_engine, + {"id": 0, "temp": "24.0", "tray_exist_bits": "1", "tray": [{"tray_type": "PLA"}]}, + ) + finally: + main._ams_index_only_logged.clear() + + assert not [r for r in caplog.records if "humidity index" in r.getMessage()] + + +@pytest.mark.asyncio +async def test_a_zero_percent_reading_is_stored_in_both_columns(db_session, test_engine): + """``float(raw) if raw else None`` stored NULL for a genuine 0%, while the + same pass wrote 0.0 to ``humidity`` -- one row disagreeing with itself. + + Numeric 0, not "0": the truthiness test only swallowed the reading when the + firmware sent the value as a number, which is how the defect survived the + string-valued samples every other test here uses.""" + printer = await _printer(db_session, "S-3140d") + + await _run_one_pass( + test_engine, + { + "id": 0, + "humidity": "5", + "humidity_raw": 0, + "temp": "24.0", + "tray_exist_bits": "1", + "tray": [{"tray_type": "PLA"}], + }, + ) + + rows = await _rows(db_session, printer.id) + assert len(rows) == 1 + assert rows[0].humidity == 0.0 + assert rows[0].humidity_raw == 0.0 + + +@pytest.mark.asyncio +async def test_a_reported_percentage_still_alarms(db_session, test_engine): + """The supported path, asserted alongside the guard so a regression that + silenced every humidity alarm could not pass as the fix.""" + printer = await _printer(db_session, "S-3140e") + + service = await _run_one_pass( + test_engine, + { + "id": 0, + "humidity": "1", + "humidity_raw": "80", + "temp": "24.0", + "tray_exist_bits": "1", + "tray": [{"tray_type": "PLA"}], + }, + ) + + service.on_ams_humidity_high.assert_awaited_once() + assert service.on_ams_humidity_high.await_args.args[3] == 80.0 + rows = await _rows(db_session, printer.id) + assert rows[0].humidity == 80.0