Fix restart bugs and harden config write path

- Send SIGUSR1 instead of SIGTERM from the web UI restart handler so
  the process reloads config in-place instead of exiting.
- Re-derive device_types, device_ids and skip_test after reloading cfg
  on restart so changed DEVICE_TYPE / DEVICE_IDS take effect.
- Hold _CONFIG_WRITE_LOCK for the entire read-modify-write transaction
  to prevent concurrent requests from clobbering each other.
- Reject duplicate section names in the order list.

Author: tomquist

src/astrameter/health_service.py

@@ -189,7 +189,7 @@ async def _handle_api_config_post(self, request):
             )
 
     async def _handle_api_restart(self, request):
-        """Acknowledge POST /api/restart and schedule a graceful shutdown via SIGTERM."""
+        """Acknowledge POST /api/restart and schedule an in-process restart via SIGUSR1."""
         import signal
 
         response = web.Response(
@@ -199,10 +199,10 @@ async def _handle_api_restart(self, request):
             content_type="application/json",
         )
         logger.info("Restart requested via web UI")
-        # Send SIGTERM to ourselves so the asyncio main loop handles graceful
-        # shutdown (finally blocks, powermeter/MQTT cleanup) before the
-        # external supervisor (Docker, systemd, HA addon) restarts the process.
-        threading.Timer(0.5, lambda: os.kill(os.getpid(), signal.SIGTERM)).start()
+        # Send SIGUSR1 so the handler in main.py sets restart_requested=True
+        # before raising KeyboardInterrupt, causing the outer loop to reload
+        # the config and re-run instead of exiting.
+        threading.Timer(0.5, lambda: os.kill(os.getpid(), signal.SIGUSR1)).start()
         return response
 
     async def _handle_not_found(self, request):

src/astrameter/main.py

@@ -390,6 +390,65 @@ async def async_main(
                 logger.exception("Error stopping health check service")
 
 
+def _resolve_device_config(
+    cfg: configparser.ConfigParser, args: argparse.Namespace
+) -> tuple[list[str], list[str], bool]:
+    """Derive device_types, device_ids and skip_test from *cfg* and CLI *args*."""
+    device_types = (
+        args.device_types
+        if args.device_types is not None
+        else [
+            dt.strip()
+            for dt in cfg.get("GENERAL", "DEVICE_TYPE", fallback="shellypro3em").split(
+                ","
+            )
+            if dt.strip()
+        ]
+    )
+    skip_test = (
+        args.skip_powermeter_test
+        if args.skip_powermeter_test is not None
+        else cfg.getboolean("GENERAL", "SKIP_POWERMETER_TEST", fallback=False)
+    )
+
+    device_ids: list[str] = list(args.device_ids) if args.device_ids is not None else []
+    if not device_ids:
+        cfg_device_ids = cfg.get("GENERAL", "DEVICE_IDS", fallback="").strip()
+        if cfg_device_ids:
+            device_ids = [
+                did.strip() for did in cfg_device_ids.split(",") if did.strip()
+            ]
+    while len(device_ids) < len(device_types):
+        device_type = device_types[len(device_ids)]
+        if device_type in ["shellypro3em", "shellyemg3", "shellyproem50"]:
+            device_ids.append(f"{device_type}-ec4609c439c{len(device_ids) + 1}")
+        else:
+            device_ids.append(f"device-{len(device_ids) + 1}")
+
+    if "shellypro3em" in device_types:
+        shellypro3em_index = device_types.index("shellypro3em")
+        device_types[shellypro3em_index] = "shellypro3em_old"
+        device_types.append("shellypro3em_new")
+        device_ids.append(device_ids[shellypro3em_index])
+
+    ct_ports = []
+    for device_type in device_types:
+        if device_type in ["ct002", "ct003"]:
+            section = get_ct_section(device_type, cfg)
+            ct_ports.append(cfg.getint(section, "UDP_PORT", fallback=UDP_PORT))
+    if len(ct_ports) != len(set(ct_ports)):
+        raise ValueError(
+            "Multiple CT002/CT003 devices are configured with the same UDP port. "
+            "Set UDP_PORT in [CT002]/[CT003] to avoid conflicts."
+        )
+
+    logger.info(f"Device Types: {device_types}")
+    logger.info(f"Device IDs: {device_ids}")
+    logger.info(f"Skip Test: {skip_test}")
+
+    return device_types, device_ids, skip_test
+
+
 def main():
     parser = argparse.ArgumentParser(description="Power meter device emulator")
     parser.add_argument(
@@ -444,61 +503,7 @@ def main():
             "Git commit not logged (set GIT_COMMIT_SHA at image build for CI images)"
         )
 
-    # Load general settings
-    device_types = (
-        args.device_types
-        if args.device_types is not None
-        else [
-            dt.strip()
-            for dt in cfg.get("GENERAL", "DEVICE_TYPE", fallback="shellypro3em").split(
-                ","
-            )
-            if dt.strip()
-        ]
-    )
-    skip_test = (
-        args.skip_powermeter_test
-        if args.skip_powermeter_test is not None
-        else cfg.getboolean("GENERAL", "SKIP_POWERMETER_TEST", fallback=False)
-    )
-
-    device_ids = args.device_ids if args.device_ids is not None else []
-    # Load device IDs from config if not provided via CLI
-    if not device_ids:
-        cfg_device_ids = cfg.get("GENERAL", "DEVICE_IDS", fallback="").strip()
-        if cfg_device_ids:
-            device_ids = [
-                did.strip() for did in cfg_device_ids.split(",") if did.strip()
-            ]
-    # Fill missing device IDs with default format
-    while len(device_ids) < len(device_types):
-        device_type = device_types[len(device_ids)]
-        if device_type in ["shellypro3em", "shellyemg3", "shellyproem50"]:
-            device_ids.append(f"{device_type}-ec4609c439c{len(device_ids) + 1}")
-        else:
-            device_ids.append(f"device-{len(device_ids) + 1}")
-
-    # For backward compatibility, replace shellypro3em with shellypro3em_old and shellypro3em_new
-    if "shellypro3em" in device_types:
-        shellypro3em_index = device_types.index("shellypro3em")
-        device_types[shellypro3em_index] = "shellypro3em_old"
-        device_types.append("shellypro3em_new")
-        device_ids.append(device_ids[shellypro3em_index])
-
-    ct_ports = []
-    for device_type in device_types:
-        if device_type in ["ct002", "ct003"]:
-            section = get_ct_section(device_type, cfg)
-            ct_ports.append(cfg.getint(section, "UDP_PORT", fallback=UDP_PORT))
-    if len(ct_ports) != len(set(ct_ports)):
-        raise ValueError(
-            "Multiple CT002/CT003 devices are configured with the same UDP port. "
-            "Set UDP_PORT in [CT002]/[CT003] to avoid conflicts."
-        )
-
-    logger.info(f"Device Types: {device_types}")
-    logger.info(f"Device IDs: {device_ids}")
-    logger.info(f"Skip Test: {skip_test}")
+    device_types, device_ids, skip_test = _resolve_device_config(cfg, args)
 
     # Apply command line throttling override if specified
     if args.throttle_interval is not None:
@@ -586,6 +591,7 @@ def _restart_handler(signum, frame):
             logger.info("Restarting service…")
             cfg = configparser.ConfigParser(dict_type=OrderedDict, interpolation=None)
             cfg.read(args.config)
+            device_types, device_ids, skip_test = _resolve_device_config(cfg, args)
         except RuntimeError as exc:
             logger.error("%s", exc)
             exit(1)

src/astrameter/web_config.py

@@ -132,6 +132,8 @@ def _validate_config_payload(sections: dict, order: list) -> None:
     """Raise ValueError if any section name, key, or value would corrupt the INI."""
     if not isinstance(order, list) or any(not isinstance(s, str) for s in order):
         raise ValueError("'order' must be a list of section names")
+    if len(order) != len(set(order)):
+        raise ValueError("'order' contains duplicate section names")
     for section, pairs in sections.items():
         if (
             not isinstance(section, str)
@@ -164,37 +166,39 @@ def write_config_from_dict(config_path: str, sections: dict, order: list) -> Non
     _validate_config_payload(sections, order)
     write_order = list(order) + [s for s in sections if s not in order]
 
-    updater = ConfigUpdater()
-    updater.optionxform = str  # type: ignore[assignment]  # preserve key case
-
-    if os.path.exists(config_path):
-        updater.read(config_path)
-
-    # Update existing sections and add new keys / remove stale keys.
-    for section_name, new_pairs in sections.items():
-        if updater.has_section(section_name):
-            for key in set(updater.options(section_name)) - new_pairs.keys():
-                updater.remove_option(section_name, key)
-        else:
-            updater.add_section(section_name)
-        for key, value in new_pairs.items():
-            updater.set(section_name, key, value)
-
-    # Remove sections not present in the incoming payload.
-    for section_name in list(updater.sections()):
-        if section_name not in sections:
-            updater.remove_section(section_name)
-
-    # Re-order sections to match *write_order* by rebuilding from
-    # detached copies.  Only needed when the order actually differs.
-    current_order = updater.sections()
-    desired = [s for s in write_order if s in sections]
-    if current_order != desired:
-        detached = {name: updater[name].detach() for name in list(updater.sections())}
-        for name in desired:
-            updater.add_section(detached[name])
-
     with _CONFIG_WRITE_LOCK:
+        updater = ConfigUpdater()
+        updater.optionxform = str  # type: ignore[assignment]  # preserve key case
+
+        if os.path.exists(config_path):
+            updater.read(config_path)
+
+        # Update existing sections and add new keys / remove stale keys.
+        for section_name, new_pairs in sections.items():
+            if updater.has_section(section_name):
+                for key in set(updater.options(section_name)) - new_pairs.keys():
+                    updater.remove_option(section_name, key)
+            else:
+                updater.add_section(section_name)
+            for key, value in new_pairs.items():
+                updater.set(section_name, key, value)
+
+        # Remove sections not present in the incoming payload.
+        for section_name in list(updater.sections()):
+            if section_name not in sections:
+                updater.remove_section(section_name)
+
+        # Re-order sections to match *write_order* by rebuilding from
+        # detached copies.  Only needed when the order actually differs.
+        current_order = updater.sections()
+        desired = [s for s in write_order if s in sections]
+        if current_order != desired:
+            detached = {
+                name: updater[name].detach() for name in list(updater.sections())
+            }
+            for name in desired:
+                updater.add_section(detached[name])
+
         _atomic_write_lines(config_path, [str(updater)])
 
 

src/astrameter/web_config_test.py

@@ -82,6 +82,11 @@ def test_validate_value_with_newline():
         _validate_config_payload({"S": {"k": "a\nb"}}, ["S"])
 
 
+def test_validate_duplicate_order():
+    with pytest.raises(ValueError, match="duplicate section names"):
+        _validate_config_payload({"S": {"k": "v"}}, ["S", "S"])
+
+
 # ---------- write_config_from_dict — new file ----------