Jelajahi Sumber

Refuse null for number and on/off settings, and read bad rows as the default (issue #2518)

A PUT /settings with null for a number setting stored the literal
"None". From then on int()/float() raised inside the response builder,
and every settings read returned 503 until the row was fixed by hand.

Explicit null for any boolean or numeric setting is now refused with a
422 that names the keys, and nothing in that request is saved. A numeric
row that does not parse reads back as its default with a warning, so an
install that already stored one recovers. The typed-key lists moved to
module constants so the save check and the read path share them.
maziggy 21 jam lalu
induk
melakukan
0cc7cf0f36

+ 2 - 0
CHANGELOG.md

@@ -5,6 +5,7 @@ All notable changes to Bambuddy will be documented in this file.
 ## [1.2.6b1] - Unreleased
 
 ### Added
+- **Ambient drying can wait until the humidity stays high, so opening the AMS lid no longer buys a drying cycle (#2518, requested by @ryansouza, contributed by @M2ABRAMSTANK in #2895)** — Ambient auto-drying started on the first humidity reading above the threshold. Opening the AMS lid to swap a spool lets room air reach the sensor for a few minutes, and that brief spike was enough to start a cycle of up to 12 hours. A new **Require sustained humidity** switch under Settings → Print Queue → Auto-Drying, shown while ambient drying is on, sets how long the humidity has to stay above the threshold before a cycle starts: 5 to 240 minutes, 15 when first switched on. It is off by default, so nothing changes until it is turned on. The 15 minutes come from a measured trace on an H2D: after the lid had been open for one to five minutes, every unit was back at or below 25% within about 12 minutes of opening, and the wait only starts counting at the first high reading. One reading at or below the threshold starts the wait over, and so does a gap of more than two minutes without a reading (a disconnect, or a print that pauses checking), which is logged with the measured gap. A missing reading changes nothing. A printer with a scheduled print pending still starts at once, for every AMS unit on it, because that drying has a deadline. The wait also applies to a printer that is printing with **Continue drying while printing** on, but not while ambient drying itself is off. It runs alongside the cooldown after a cycle instead of being added to it, never interrupts a running or manual cycle, and does not log a wait for a unit the printer refuses to dry. Translated in all locales.
 - **Other applications can send messages through your notification channels** — A new switch per notification provider, **Messages from connected apps** (off by default), lets an application such as Bambuddy Orders send its own messages ("3 orders need you") to Telegram, ntfy, email and the other channels. The app calls `POST /api/v1/notifications/app-message` with an API key that has the new **Send Notifications** permission (off by default; the key's owner also needs `notifications:update`). Messages go through quiet hours, the daily digest and the notification log like Bambuddy's own. Text is plain, links must be http(s), and each key is limited to 20 messages a minute. `GET /api/v1/notifications/app-message/channels` tells the app which channels take its messages. The check behind the electricity-price door and this one is now one shared helper.
 - **A link can open one batch order** — `/queue?batch=<id>` opens the Batches tab with that order highlighted and scrolled into view, whatever its status.
 - **The streaming overlay can show the printer model (#3080, requested and contributed by @adamspicedev in #3099)** — Settings → API Keys → Streaming Overlay has a new **Printer model** checkbox, which adds `model` to the overlay URL's `show=` list. It can be picked together with the printer name or on its own. With both, the overlay shows `Big Mumma · H2D`; with only the model, `H2D`. A printer with no model stored shows the name alone, without a stray separator. The model is off by default, so every overlay URL already set up in OBS looks the same after upgrading. The token-authenticated feed that OBS reads (`/printers/{id}/overlay-status`) now returns `model` for connected and disconnected printers; it exposes nothing else new. A long name plus model in a narrow OBS source is now cut off with an ellipsis instead of running past the edge of the panel.
@@ -44,6 +45,7 @@ All notable changes to Bambuddy will be documented in this file.
 - **The frontend build no longer warns about `path` and `crypto` being externalized for the STEP previewer (#2976)** — `occt-import-js`, the Emscripten build behind STEP previews, requires both modules, but only inside its `ENVIRONMENT_IS_NODE` branches; in the browser it loads its `.wasm` from the URL the preview worker passes and draws randomness from `crypto.getRandomValues`. Vite still externalized both and printed two warnings on every build. `vite.config.ts` now drops exactly those two warnings for that one package through `build.rolldownOptions.onLog`, so an externalization anywhere else, or of any other module, still shows.
 
 ### Fixed
+- **Saving an empty value for a number setting through the API no longer breaks the Settings page** — A direct API call, script or Home Assistant `rest_command` could send `null` for a number setting such as the keep-warm bed temperature. Bambuddy stored it as the text "None", and from then on every load of the settings failed with an error, for every setting on the page, until the database was fixed by hand. A `null` for a setting that can't be empty, whether a number or an on/off switch, is now refused with a message that names the setting, and nothing from that request is saved. A setting already stored that way now loads as its default, so an affected install recovers on its own.
 - **"Reset usage to 0" on a Spoolman spool set it back to full (#2906, reported and contributed by @ojimpo in #2939)** — The reset set Spoolman's `used_weight` to 0. Spoolman calculates the remaining weight as initial minus used, so the spool jumped back to full and its measured remaining weight was lost: a roll with 737 g left read 1000 g, and later prints were charged against filament that wasn't there. The confirmation dialog promised the opposite, that the spool and its remaining weight would not change. The reset now stores the used weight at that moment in a Bambuddy field in the spool's custom fields (`bambu_weight_used_baseline`), the same way it has always worked without Spoolman, and Bambuddy subtracts it when it shows the consumed counter. Spoolman's initial, used and remaining weights are left untouched, and so is every other custom field. The value is stored as text because Spoolman sets a field's type on first use and never lets it change; a number would have been rejected on every install. The bulk reset works the same way. Spools already reset by the old version stay as they are: their remaining weight was overwritten in Spoolman, so they need re-weighing.
 - **Filament printed from the external spool was deducted from an AMS spool (#3166)** — The firmware doesn't accept the external spool in a print command's `ams_mapping` list. So BambuStudio and Bambuddy's own dispatch both write it there as -1, the same value as an unused slot, and put the real target in `ams_mapping2`. Bambuddy read only the first list, so an external-spool print looked unmapped, and usage tracking fell back to guessing by position: the first loaded AMS slot. The reporter's H2D charged every external-spool print that way, including 225 g of ABS taken from the PLA spool in AMS slot 1; the same was reported on H2S and P2S. The external spool is now read from `ams_mapping2` (left or right on dual-nozzle printers). A slot the mapping marks as unused is no longer given a tray by position. When a mapping names no tray at all, the tray the printer reports decides instead. The last change also stops an unused slot from taking over the external spool that another slot really used and dropping that slot's weight (#2880).
 - **Upgrading from Bambuddy 0.2.4.0 or older no longer crashes at startup** — Since 1.2.5.4, startup first converts old failure-reason labels to the new vocabulary (#2974), in both the archives and the print log. A database from before the print log had a failure-reason column (#1378) reached that conversion before the migration that adds the column, and Bambuddy stopped with `no such column: failure_reason` instead of starting. The conversion now skips a table that doesn't have the column yet; it can't hold old labels anyway. On SQLite it also rebuilds the archive search index before converting archives, because archives created before that index existed were never added to it, and updating one of them failed with `database disk image is malformed`. It only rebuilds when there is something to convert, and it skips both tables entirely when nothing needs converting, so a normal startup does less work than before.

+ 112 - 80
backend/app/api/routes/settings.py

@@ -167,6 +167,96 @@ async def set_setting(db: AsyncSession, key: str, value: str) -> None:
     await upsert_setting(db, Settings, key, value)
 
 
+# Settings stored as booleans / numbers. Storage is a VARCHAR column, so
+# _build_settings_response() parses these back, and update_settings() refuses
+# an explicit null for them: a null is stored as the literal "None", which
+# reads back as False for a boolean and is not a number at all.
+_BOOL_SETTING_KEYS = frozenset(
+    {
+        "auto_archive",
+        "save_thumbnails",
+        "capture_finish_photo",
+        "finish_photo_restore_plate",
+        "spoolman_enabled",
+        "spoolman_disable_weight_sync",
+        "spoolman_report_partial_usage",
+        "auto_add_unknown_rfid",
+        "disable_filament_warnings",
+        "prefer_lowest_filament",
+        "check_updates",
+        "check_printer_firmware",
+        "include_beta_updates",
+        "virtual_printer_enabled",
+        "ftp_retry_enabled",
+        "mqtt_enabled",
+        "mqtt_use_tls",
+        "ha_enabled",
+        "per_printer_mapping_expanded",
+        "prometheus_enabled",
+        "user_notifications_enabled",
+        "queue_drying_enabled",
+        "queue_drying_block",
+        "ambient_drying_enabled",
+        "print_drying_enabled",
+        "require_plate_clear",
+        "queue_shortest_first",
+        # default_bed_levelling / default_flow_cali / default_nozzle_offset_cali
+        # are tri-state strings (off/on/auto) — parsed via the raw-string else
+        # branch; the TriState validator coerces legacy "true"/"false" rows.
+        "default_vibration_cali",
+        "default_layer_inspect",
+        "default_timelapse",
+        "default_confirm_outcome",
+        "confirm_outcome_external_prints",
+        "confirm_default_good_on_plate_clear",
+        "billing_enabled",
+        "printer_kill_switch_enabled",
+        "ldap_enabled",
+        "ldap_auto_provision",
+        "local_login_enabled",
+        "preheat_enabled",
+        "queue_keep_bed_warm",
+    }
+)
+
+_FLOAT_SETTING_KEYS = frozenset(
+    {
+        "default_filament_cost",
+        "energy_cost_per_kwh",
+        "ams_temp_good",
+        "ams_temp_fair",
+        "library_disk_warning_gb",
+        "low_stock_threshold",
+    }
+)
+
+_INT_SETTING_KEYS = frozenset(
+    {
+        "ams_humidity_good",
+        "ams_humidity_fair",
+        "ams_history_retention_days",
+        "printer_sensor_history_retention_days",
+        "ftp_retry_count",
+        "ftp_retry_delay",
+        "ftp_timeout",
+        "mqtt_port",
+        "stagger_group_size",
+        "stagger_interval_minutes",
+        "forecast_global_lead_time_days",
+        "location_sensor_poll_interval",
+        "finance_budget_reset_day",
+        "session_max_hours",
+        "pipeline_max_copies",
+        "preheat_max_wait_seconds",
+        "preheat_soak_seconds",
+        "queue_keep_warm_bed_temp",
+        "queue_keep_warm_max_minutes",
+        "queue_max_concurrent_uploads",
+        "ambient_drying_sustained_minutes",
+    }
+)
+
+
 async def _build_settings_response(db: AsyncSession, is_api_key: bool = False) -> AppSettings:
     """Build the full settings response, scrubbing secrets for API-key callers."""
     settings_dict = DEFAULT_SETTINGS.model_dump()
@@ -175,95 +265,27 @@ async def _build_settings_response(db: AsyncSession, is_api_key: bool = False) -
     for setting in result.scalars().all():
         if setting.key not in settings_dict:
             continue
-        if setting.key in [
-            "auto_archive",
-            "save_thumbnails",
-            "capture_finish_photo",
-            "finish_photo_restore_plate",
-            "spoolman_enabled",
-            "spoolman_disable_weight_sync",
-            "spoolman_report_partial_usage",
-            "auto_add_unknown_rfid",
-            "disable_filament_warnings",
-            "prefer_lowest_filament",
-            "check_updates",
-            "check_printer_firmware",
-            "include_beta_updates",
-            "virtual_printer_enabled",
-            "ftp_retry_enabled",
-            "mqtt_enabled",
-            "mqtt_use_tls",
-            "ha_enabled",
-            "per_printer_mapping_expanded",
-            "prometheus_enabled",
-            "user_notifications_enabled",
-            "queue_drying_enabled",
-            "queue_drying_block",
-            "ambient_drying_enabled",
-            "print_drying_enabled",
-            "require_plate_clear",
-            "queue_shortest_first",
-            # default_bed_levelling / default_flow_cali / default_nozzle_offset_cali
-            # are tri-state strings (off/on/auto) — parsed via the raw-string else
-            # branch; the TriState validator coerces legacy "true"/"false" rows.
-            "default_vibration_cali",
-            "default_layer_inspect",
-            "default_timelapse",
-            "default_confirm_outcome",
-            "confirm_outcome_external_prints",
-            "confirm_default_good_on_plate_clear",
-            "billing_enabled",
-            "printer_kill_switch_enabled",
-            "ldap_enabled",
-            "ldap_auto_provision",
-            "local_login_enabled",
-            "preheat_enabled",
-            "queue_keep_bed_warm",
-        ]:
+        if setting.key in _BOOL_SETTING_KEYS:
             settings_dict[setting.key] = setting.value.lower() == "true"
-        elif setting.key in [
-            "default_filament_cost",
-            "energy_cost_per_kwh",
-            "ams_temp_good",
-            "ams_temp_fair",
-            "library_disk_warning_gb",
-            "low_stock_threshold",
-        ]:
-            settings_dict[setting.key] = float(setting.value)
+        elif setting.key in _FLOAT_SETTING_KEYS or setting.key in _INT_SETTING_KEYS:
+            # A value that does not parse (the literal "None" from an old
+            # null save, or a hand-edited row) keeps the default instead of
+            # taking the whole settings response down with it.
+            parse = int if setting.key in _INT_SETTING_KEYS else float
+            try:
+                settings_dict[setting.key] = parse(setting.value)
+            except (TypeError, ValueError):
+                logger.warning("Setting %s has an unparseable value; using the default", setting.key)
         elif setting.key in [
             # Nullable floats. Settings storage stringifies None to the literal
-            # "None", so these cannot go in the list above -- float("None")
-            # raises and would take the whole settings response with it (#2905).
+            # "None", which must read back as null here -- not as the default
+            # the _FLOAT_SETTING_KEYS branch above falls back to (#2905).
             "ams_temp_alarm",
         ]:
             try:
                 settings_dict[setting.key] = float(setting.value)
             except (TypeError, ValueError):
                 settings_dict[setting.key] = None
-        elif setting.key in [
-            "ams_humidity_good",
-            "ams_humidity_fair",
-            "ams_history_retention_days",
-            "printer_sensor_history_retention_days",
-            "ftp_retry_count",
-            "ftp_retry_delay",
-            "ftp_timeout",
-            "mqtt_port",
-            "stagger_group_size",
-            "stagger_interval_minutes",
-            "forecast_global_lead_time_days",
-            "location_sensor_poll_interval",
-            "finance_budget_reset_day",
-            "session_max_hours",
-            "pipeline_max_copies",
-            "preheat_max_wait_seconds",
-            "preheat_soak_seconds",
-            "queue_keep_warm_bed_temp",
-            "queue_keep_warm_max_minutes",
-            "queue_max_concurrent_uploads",
-            "ambient_drying_sustained_minutes",
-        ]:
-            settings_dict[setting.key] = int(setting.value)
         elif setting.key == "default_printer_id":
             settings_dict[setting.key] = int(setting.value) if setting.value and setting.value != "None" else None
         elif setting.key == "open_in_slicer":
@@ -308,6 +330,16 @@ async def update_settings(
     """Update application settings."""
     update_data = settings_update.model_dump(exclude_unset=True)
 
+    # An explicit null for a boolean or numeric setting has no meaning -- these
+    # are not clearable -- and would be stored as the literal "None".
+    null_keys = sorted(
+        key
+        for key, value in update_data.items()
+        if value is None and key in (_BOOL_SETTING_KEYS | _FLOAT_SETTING_KEYS | _INT_SETTING_KEYS)
+    )
+    if null_keys:
+        raise HTTPException(status_code=422, detail=f"These settings cannot be null: {', '.join(null_keys)}")
+
     # Safety refusals on disabling local login (#1589). Two failure modes
     # would otherwise lock everyone out of the install:
     #   1. No enabled OIDC provider exists — nobody could authenticate.

+ 44 - 0
backend/tests/integration/test_settings_api.py

@@ -61,6 +61,50 @@ class TestSettingsAPI:
         assert response.status_code == 200
         assert response.json()["ams_temp_alarm"] is None
 
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    @pytest.mark.parametrize(
+        ("key", "default"),
+        [
+            ("ambient_drying_sustained_minutes", 0),
+            ("queue_keep_warm_bed_temp", 90),
+            ("default_filament_cost", 25.0),
+        ],
+    )
+    async def test_unparseable_number_setting_reads_back_as_the_default(
+        self, async_client: AsyncClient, db_session, key, default
+    ):
+        """A numeric row holding "None" (an old null save) or any other
+        unparseable value must fall back to the default, not make int()/float()
+        raise inside the response builder and take every setting with it."""
+        from backend.app.models.settings import Settings
+
+        db_session.add(Settings(key=key, value="None"))
+        await db_session.commit()
+
+        response = await async_client.get("/api/v1/settings/")
+
+        assert response.status_code == 200
+        assert response.json()[key] == default
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    @pytest.mark.parametrize("key", ["ambient_drying_sustained_minutes", "default_filament_cost", "auto_archive"])
+    async def test_null_for_a_typed_setting_is_refused_and_not_stored(self, async_client: AsyncClient, key):
+        """A null for a boolean or numeric setting would be stored as the literal
+        "None". The whole request is refused, so a valid field sent alongside it
+        is not half-applied either."""
+        before = (await async_client.get("/api/v1/settings/")).json()
+        assert before["currency"] != "EUR"
+
+        response = await async_client.put("/api/v1/settings/", json={key: None, "currency": "EUR"})
+
+        assert response.status_code == 422
+        assert key in response.json()["detail"]
+        after = (await async_client.get("/api/v1/settings/")).json()
+        assert after[key] == before[key]
+        assert after["currency"] == before["currency"]
+
     @pytest.mark.asyncio
     @pytest.mark.integration
     async def test_a_set_temp_alarm_reads_back_as_a_float(self, async_client: AsyncClient, db_session):