diff --git a/ofm_config_full.json b/ofm_config_full.json index 38a76685..1cfedb26 100644 --- a/ofm_config_full.json +++ b/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": { diff --git a/ofm_config_simulation.json b/ofm_config_simulation.json index 9acf747b..a254fa85 100644 --- a/ofm_config_simulation.json +++ b/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": { diff --git a/src/openflexure_microscope_server/things/settings_manager.py b/src/openflexure_microscope_server/things/settings_manager.py deleted file mode 100644 index df0e144d..00000000 --- a/src/openflexure_microscope_server/things/settings_manager.py +++ /dev/null @@ -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, - } diff --git a/src/openflexure_microscope_server/things/system.py b/src/openflexure_microscope_server/things/system.py new file mode 100644 index 00000000..12aaadf9 --- /dev/null +++ b/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, + } diff --git a/src/openflexure_microscope_server/things/system_control.py b/src/openflexure_microscope_server/things/system_control.py deleted file mode 100644 index 82dc276a..00000000 --- a/src/openflexure_microscope_server/things/system_control.py +++ /dev/null @@ -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) diff --git a/tests/test_system_thing.py b/tests/test_system_thing.py new file mode 100644 index 00000000..d1fb4a1e --- /dev/null +++ b/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." diff --git a/webapp/src/App.vue b/webapp/src/App.vue index 97c46c19..3f76338e 100644 --- a/webapp/src/App.vue +++ b/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 { diff --git a/webapp/src/components/tabContentComponents/aboutComponents/statusPane.vue b/webapp/src/components/tabContentComponents/aboutComponents/statusPane.vue index 6314666f..4d8d032d 100644 --- a/webapp/src/components/tabContentComponents/aboutComponents/statusPane.vue +++ b/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; diff --git a/webapp/src/components/tabContentComponents/powerContent.vue b/webapp/src/components/tabContentComponents/powerContent.vue index 855174d8..a865e74f 100644 --- a/webapp/src/components/tabContentComponents/powerContent.vue +++ b/webapp/src/components/tabContentComponents/powerContent.vue @@ -1,23 +1,23 @@