Merge branch 'merge-settings-and-system-control' into 'v3'

Merge settings and system control

See merge request openflexure/openflexure-microscope-server!323

ofm_config_full.json

@@ -5,8 +5,7 @@
         "/auto_recentre_stage/": "openflexure_microscope_server.things.auto_recentre_stage:RecentringThing",
         "/autofocus/": "openflexure_microscope_server.things.autofocus:AutofocusThing",
         "/camera_stage_mapping/": "openflexure_microscope_server.things.camera_stage_mapping:CameraStageMapper",
-        "/system_control/": "openflexure_microscope_server.things.system_control:SystemControlThing",
-        "/settings/": "openflexure_microscope_server.things.settings_manager:SettingsManager",
+        "/system/": "openflexure_microscope_server.things.system:OpenFlexureSystem",
         "/smart_scan/": {
             "class": "openflexure_microscope_server.things.smart_scan:SmartScanThing",
             "kwargs": {

ofm_config_simulation.json

@@ -5,8 +5,7 @@
         "/auto_recentre_stage/": "openflexure_microscope_server.things.auto_recentre_stage:RecentringThing",
         "/autofocus/": "openflexure_microscope_server.things.autofocus:AutofocusThing",
         "/camera_stage_mapping/": "openflexure_microscope_server.things.camera_stage_mapping:CameraStageMapper",
-        "/system_control/": "openflexure_microscope_server.things.system_control:SystemControlThing",
-        "/settings/": "openflexure_microscope_server.things.settings_manager:SettingsManager",
+        "/system/": "openflexure_microscope_server.things.system:OpenFlexureSystem",
         "/smart_scan/": {
             "class": "openflexure_microscope_server.things.smart_scan:SmartScanThing",
             "kwargs": {

src/openflexure_microscope_server/things/settings_manager.py

@@ -1,74 +0,0 @@
-"""OpenFlexure Settings Management.
-
-This module provides some settings management across the other Things, and
-for code that currently lives in clients but needs to persist settings on
-the server.
-"""
-
-from collections.abc import Mapping
-from socket import gethostname
-from typing import Optional
-from uuid import UUID, uuid4
-
-import labthings_fastapi as lt
-
-from openflexure_microscope_server.utilities import VersionData, robust_version_strings
-
-
-class SettingsManager(lt.Thing):
-    """Provides functionality to other Things about the current server state.
-
-    The SettingsManager is used to get information the microscope ID, the hostname
-    and the state of other Things.
-    """
-
-    _microscope_id: Optional[str] = None
-
-    @lt.thing_setting
-    def microscope_id(self) -> UUID:
-        """A unique identifier for this microscope."""
-        if self._microscope_id is None:
-            self._microscope_id = str(uuid4())
-        return UUID(self._microscope_id)
-
-    @microscope_id.setter
-    def microscope_id(self, uuid: UUID):
-        # TODO make read only but still settable from disk
-        self._microscope_id = uuid
-
-    @lt.thing_property
-    def hostname(self) -> str:
-        """The hostname of the microscope, as reported by its operating system."""
-        return gethostname()
-
-    _version_data: Optional[VersionData] = None
-
-    @lt.thing_property
-    def version_data(self) -> VersionData:
-        """The version string and version source for the server.
-
-        The source may be a commit hash if installed from git. "TOML" if installed from
-        source, or "Dist" if installed from a distribution package.
-
-        If the version or its source cannot be determined an error will be Logged.
-        """
-        # Don't save as a setting as it may change. Get the version string the first
-        # time the property is requested this boot.
-        if self._version_data is None:
-            self._version_data = robust_version_strings()
-        return self._version_data
-
-    @lt.thing_action
-    def get_things_state(self, metadata_getter: lt.deps.GetThingStates) -> Mapping:
-        """Metadata summarising the current state of all Things in the server."""
-        return metadata_getter()
-
-    @property
-    def thing_state(self) -> Mapping:
-        """Summary metadata describing the current state of the Thing."""
-        return {
-            "hostname": self.hostname,
-            "microscope-uuid": str(self.microscope_id),
-            "version": self.version_data.version,
-            "version_source": self.version_data.version_source,
-        }

src/openflexure_microscope_server/things/system.py

@@ -0,0 +1,137 @@
+"""OpenFlexure System.
+
+A module to control the underlying microscope system and to expose information about
+the microscope, server, and thing states to the web API.
+"""
+
+from collections.abc import Mapping
+import socket
+from typing import Optional
+from uuid import UUID, uuid4
+import subprocess
+import os
+from signal import SIGTERM
+import time
+
+from pydantic import BaseModel
+
+import labthings_fastapi as lt
+
+from openflexure_microscope_server.utilities import VersionData, robust_version_strings
+
+SHUTDOWN_CMD = ["sudo", "shutdown", "-h", "now"]
+REBOOT_CMD = ["sudo", "shutdown", "-r", "now"]
+
+
+class CommandOutput(BaseModel):
+    """A pydantic model passing the STDOUT and STDERR from a subprocess over HTTP."""
+
+    output: str
+    error: str
+
+
+class OpenFlexureSystem(lt.Thing):
+    """Describe and control the OpenFlexure system.
+
+    This Thing:
+
+    * Exposes information about the Microscope, Server, and Thing states to the web
+        API.
+    * Controls the underlying OS on the Raspberry Pi allowing shutdown and restarting
+        of the system.
+    """
+
+    _microscope_id: Optional[str] = None
+
+    @lt.thing_setting
+    def microscope_id(self) -> UUID:
+        """A unique identifier for this microscope."""
+        if self._microscope_id is None:
+            self._microscope_id = str(uuid4())
+        return UUID(self._microscope_id)
+
+    @microscope_id.setter
+    def microscope_id(self, uuid: UUID):
+        # TODO make read only but still settable from disk
+        self._microscope_id = uuid
+
+    @lt.thing_property
+    def hostname(self) -> str:
+        """The hostname of the microscope, as reported by its operating system."""
+        return socket.gethostname()
+
+    _version_data: Optional[VersionData] = None
+
+    @lt.thing_property
+    def version_data(self) -> VersionData:
+        """The version string and version source for the server.
+
+        The source may be a commit hash if installed from git. "TOML" if installed from
+        source, or "Dist" if installed from a distribution package.
+
+        If the version or its source cannot be determined an error will be Logged.
+        """
+        # Don't save as a setting as it may change. Get the version string the first
+        # time the property is requested this boot.
+        if self._version_data is None:
+            self._version_data = robust_version_strings()
+        return self._version_data
+
+    @lt.thing_property
+    def is_raspberrypi(self) -> bool:
+        """Return True if running on a Raspberry Pi."""
+        return os.path.exists("/usr/bin/raspi-config")
+
+    @lt.thing_action
+    def shutdown(self) -> CommandOutput:
+        """Attempt to shutdown the device."""
+        if not self.is_raspberrypi:
+            os.kill(os.getpid(), SIGTERM)
+            time.sleep(0.5)
+            # If this line is reached then the server did not shut down!
+            return CommandOutput(
+                output="",
+                error="Shutdown command sent to server process, but the server has not shutdown.",
+            )
+
+        # On a Raspberry Pi
+        p = subprocess.Popen(
+            SHUTDOWN_CMD,
+            stderr=subprocess.PIPE,
+            stdout=subprocess.PIPE,
+        )
+
+        out, err = p.communicate()
+        return CommandOutput(output=out, error=err)
+
+    @lt.thing_action
+    def reboot(self) -> CommandOutput:
+        """Attempt to reboot the device."""
+        if not self.is_raspberrypi:
+            return CommandOutput(
+                output="",
+                error="Restart is only available on Raspberry Pi.",
+            )
+
+        p = subprocess.Popen(
+            REBOOT_CMD,
+            stderr=subprocess.PIPE,
+            stdout=subprocess.PIPE,
+        )
+        out, err = p.communicate()
+        return CommandOutput(output=out, error=err)
+
+    @lt.thing_action
+    def get_things_state(self, metadata_getter: lt.deps.GetThingStates) -> Mapping:
+        """Metadata summarising the current state of all Things in the server."""
+        return metadata_getter()
+
+    @property
+    def thing_state(self) -> Mapping:
+        """Summary metadata describing the current state of the Thing."""
+        return {
+            "hostname": self.hostname,
+            "microscope-uuid": str(self.microscope_id),
+            "version": self.version_data.version,
+            "version_source": self.version_data.version_source,
+        }

src/openflexure_microscope_server/things/system_control.py

@@ -1,48 +0,0 @@
-"""OpenFlexure Microscope system control Thing.
-
-This module defines a Thing that can shut down or restart the host computer.
-"""
-
-import subprocess
-import os
-import labthings_fastapi as lt
-from pydantic import BaseModel
-
-
-class CommandOutput(BaseModel):
-    """A pydantic model passing the STDOUT and STDERR from a subprocess over HTTP."""
-
-    output: str
-    error: str
-
-
-class SystemControlThing(lt.Thing):
-    """Attempt to shutdown the device."""
-
-    @lt.thing_action
-    def shutdown(self) -> CommandOutput:
-        """Attempt to shutdown the device."""
-        p = subprocess.Popen(
-            ["sudo", "shutdown", "-h", "now"],
-            stderr=subprocess.PIPE,
-            stdout=subprocess.PIPE,
-        )
-
-        out, err = p.communicate()
-        return CommandOutput(output=out, error=err)
-
-    @lt.thing_property
-    def is_raspberrypi() -> bool:
-        """Return True if running on a Raspberry Pi."""
-        return os.path.exists("/usr/bin/raspi-config")
-
-    @lt.thing_action
-    def reboot(self) -> CommandOutput:
-        """Attempt to reboot the device."""
-        p = subprocess.Popen(
-            ["sudo", "shutdown", "-r", "now"],
-            stderr=subprocess.PIPE,
-            stdout=subprocess.PIPE,
-        )
-        out, err = p.communicate()
-        return CommandOutput(output=out, error=err)

tests/test_system_thing.py

@@ -0,0 +1,145 @@
+"""Test the OpenFlexureSystem Thing *without* connecting it to a LabThings Server."""
+
+import os
+from signal import SIGTERM
+from uuid import UUID
+
+from openflexure_microscope_server.things import system
+from openflexure_microscope_server.utilities import VersionData, robust_version_strings
+
+
+def _is_raspberrypi() -> bool:
+    """Check if the machine running the test is a Raspberry Pi.
+
+    This information is needed to check if the ``is_raspberrypi`` property of the Thing
+    is correct. This is implemented in a different way to in the main code.
+    """
+    # Check if the file specifying the Raspberry Pi model exists
+    if not os.path.isfile("/proc/device-tree/model"):
+        return False
+    with open("/proc/device-tree/model", "r", encoding="utf-8") as model_file:
+        model_name = model_file.read()
+    return "Raspberry Pi" in model_name
+
+
+def test_is_raspberry():
+    """Check the thing property reports whether this is a Raspberry Pi correctly."""
+    assert system.OpenFlexureSystem().is_raspberrypi == _is_raspberrypi()
+
+
+def test_version_data():
+    """Check VersionData is returned.
+
+    The content of robust_version_strings() is tested elsewhere.
+    """
+    system_thing = system.OpenFlexureSystem()
+    data = system_thing.version_data
+    assert isinstance(data, VersionData)
+    assert data == robust_version_strings()
+
+    fake_data = VersionData(version="fake", version_source="fake")
+    system_thing._version_data = fake_data
+    data = system_thing.version_data
+    assert data == fake_data
+
+
+def test_hostname(mocker):
+    """Check the hostname matches what socket.gethostname() returns."""
+    mock_gethostname = mocker.patch("socket.gethostname", return_value="foobar")
+    system_thing = system.OpenFlexureSystem()
+    assert system_thing.hostname == "foobar"
+    mock_gethostname.assert_called_once_with()
+
+
+def test_microscope_id():
+    """Check the microscope UUID is a valid UUID and doesn't change when read again."""
+    system_thing = system.OpenFlexureSystem()
+    microscope_id = system_thing.microscope_id
+    assert isinstance(microscope_id, UUID)
+    assert microscope_id == system_thing.microscope_id
+
+
+def test_thing_state(mocker):
+    """Check the thing state contains version data, hostname, and a UUID string."""
+    mocker.patch("socket.gethostname", return_value="foobar")
+    version_data = robust_version_strings()
+    system_thing = system.OpenFlexureSystem()
+    state_dict = system_thing.thing_state
+    assert state_dict["hostname"] == "foobar"
+    # Check the UUID in the dictionary is a string
+    assert isinstance(state_dict["microscope-uuid"], str)
+    # Check uuid string can be converted to valid UUID
+    UUID(state_dict["microscope-uuid"])
+    assert state_dict["version"] == version_data.version
+    assert state_dict["version_source"] == version_data.version_source
+
+
+class MockPiSystem(system.OpenFlexureSystem):
+    """A OpenFlexureSystem Thing that always claims to be a Raspberry Pi."""
+
+    is_raspberrypi: bool = True
+
+
+class MockNonPiSystem(system.OpenFlexureSystem):
+    """A OpenFlexureSystem Thing that never claims to be a Raspberry Pi."""
+
+    is_raspberrypi: bool = False
+
+
+def test_pi_shutdown(mocker):
+    """Check that on a Pi will receive the correct shutdown command."""
+    # Check the shutdown command is as expected.
+    assert system.SHUTDOWN_CMD == ["sudo", "shutdown", "-h", "now"]
+    # Mock the shutdown command as we don't want to shutdown when running tests.
+    mocker.patch.object(system, "SHUTDOWN_CMD", new=["echo", "shutdown"])
+    # Call shutdown on a MockPiSystem
+    system_thing = MockPiSystem()
+    result = system_thing.shutdown()
+
+    # Check the result of the echo mock command was returned
+    assert result.output.strip() == "shutdown"
+    assert result.error.strip() == ""
+
+
+def test_pi_reboot(mocker):
+    """Check that on a Pi will receive the correct reboot command."""
+    # Check the reboot command is as expected.
+    assert system.REBOOT_CMD == ["sudo", "shutdown", "-r", "now"]
+    # Mock the reboot command as we don't want to reboot when running tests.
+    mocker.patch.object(system, "REBOOT_CMD", new=["echo", "restart"])
+    # Call reboot on a MockPiSystem
+    system_thing = MockPiSystem()
+    result = system_thing.reboot()
+
+    # Check the result of the echo mock command was returned
+    assert result.output.strip() == "restart"
+    assert result.error.strip() == ""
+
+
+def test_non_pi_shutdown(mocker):
+    """Check that a server not on a pi runs os.kill when asked to shutdown.
+
+    It should do this instead of running the shutdown command in a subprocess.
+    """
+    # Mock os.kill
+    mock_kill = mocker.patch("os.kill")
+    # Get this subprocess
+    pid = os.getpid()
+    system_thing = MockNonPiSystem()
+    result = system_thing.shutdown()
+
+    # Check it tried to kill this process with SIGTERM
+    mock_kill.assert_called_once_with(pid, SIGTERM)
+    # Check output is an appropriate error message (that we have not shut down!)
+    assert result.output.strip() == ""
+    assert "but the server has not shutdown" in result.error
+
+
+def test_non_pi_reboot():
+    """Check that a server not on a pi refuses to restart."""
+    system_thing = MockNonPiSystem()
+    result = system_thing.reboot()
+
+    # Check output is an appropriate error message
+    assert result.output.strip() == ""
+    assert result.error.strip() == "Restart is only available on Raspberry Pi."

webapp/src/App.vue

@@ -236,7 +236,7 @@ export default {
           }
         }
         try {
-          let hostname = await this.readThingProperty("settings", "hostname");
+          let hostname = await this.readThingProperty("system", "hostname");
           this.$store.commit("changeMicroscopeHostname", hostname);
           document.title = `OpenFlexure Microscope: ${hostname}`;
         } catch {

webapp/src/components/tabContentComponents/aboutComponents/statusPane.vue

@@ -76,7 +76,7 @@ export default {
 
   async mounted() {
     let version_data = await this.readThingProperty(
-      "settings",
+      "system",
       "version_data"
     );
     this.version = version_data.version;

webapp/src/components/tabContentComponents/powerContent.vue

@@ -1,23 +1,23 @@
 <template>
   <!-- Grid managing tab content -->
-  <div class="center">
-  <h1 style="text-align: center;"> Power</h1>
-  <p style="text-align: center;"> It's essential to turn off your OpenFlexure Microscope here before unplugging it.
-  <br>
-  <br>
-  Unplugging the microscope unexpectedly can damage the SD card or onboard computer.
-  </p>
-    <div class="buttons">
+  <div class="container">
+    <h1 id="power-title"> Power</h1>
+    <p id="power-msg"> It's essential to turn off your OpenFlexure Microscope here before unplugging it.
+    <br>
+    <br>
+    Unplugging the microscope unexpectedly can damage the SD card or onboard computer.
+    </p>
+    <div class="buttons-container">
       <button
-        v-show="'shutdown' in things.system_control.actions"
-        class="uk-button uk-button-primary uk-width-1-3"
+        v-show="'shutdown' in things.system.actions"
+        class="uk-button uk-button-primary uk-width-1-3 shutdown-button"
         @click="systemRequest('shutdown')"
       >
         Shutdown
       </button>
       <button
-        v-show="'reboot' in things.system_control.actions"
-        class="uk-button uk-button-primary uk-width-1-3"
+        v-if="isRaspberrypi"
+        class="uk-button uk-button-primary uk-width-1-3 shutdown-button"
         @click="systemRequest('reboot')"
       >
         Restart
@@ -33,6 +33,12 @@ export default {
   name: "PowerContent",
 
   components: {},
+
+  data: function() {
+    return {
+      isRaspberrypi: undefined,
+    }
+  },
   
   computed: {
     things: function() {
@@ -40,8 +46,15 @@ export default {
     }
   },
 
+  async mounted() {
+    this.isRaspberrypi = await this.readThingProperty(
+      "system",
+      "is_raspberrypi"
+    );
+  },
+
   methods: {
-  systemRequest: function(action) {
+    systemRequest: function(action) {
       let message = "";
       if (action == "reboot") {
         message = "Restart microscope?"
@@ -55,7 +68,7 @@ export default {
           this.$store.commit("wot/deleteAllThingDescriptions");
           // Post and silence errors
           axios
-            .post(this.thingActionUrl("system_control", action))
+            .post(this.thingActionUrl("system", action))
             .catch(() => {});
         },
         () => {}
@@ -69,14 +82,32 @@ export default {
 // Custom UIkit CSS modifications
 @import "../../assets/less/theme.less";
 
-.center {
-    right: 50%;
-    bottom: 50%;
-    transform: translate(50%,50%);
-    position: absolute;
+
+.container {
+  display: flex;
+  flex-direction: column;
+  justify-content: center;
+  align-items: center;
+  width: 100%;
+  height: 100%;
+  padding: 2em;
+  box-sizing: border-box;
 }
 
-.uk-button {
+#power-title {
+  text-align: center;
+}
+
+#power-msg {
+  text-align: center;
+}
+
+.buttons-container {
+  width: 100%;
+  text-align: center;
+}
+
+.shutdown-button {
   display: inline;
   text-align:center;
   margin: 30px 30px 30px 30px;