Merge branch 'fix-transitions-dict-updates' into 'master'

Fix AttributeError on _set_obs_state (transitions dict update)

See merge request ska-telescope/lmc-base-classes!24

.release

-release=0.6.1
-tag=lmcbaseclasses-0.6.1
+release=0.6.2
+tag=lmcbaseclasses-0.6.2

README.md

@@ -25,6 +25,11 @@ The lmc-base-classe repository contains set of eight classes as mentioned in SKA
 
 ## Version History
 
+#### 0.6.2
+- Fix issue with incorrect updates to transitions dict from inherited devices.
+  Only noticeable if running multiple devices of different types in the same
+  process.
+
 #### 0.6.1
 - Add ON state to SKABaseDeviceStateModel.
 - Move On() and Off() commands to SKABaseDevice.

src/ska/base/control_model.py

@@ -355,7 +355,8 @@ class DeviceStateModel:
         :type initial_state: a state with an entry in the transitions
             table
         """
-        self._transitions = transitions
+        # instances may update transitions dict, so need a copy
+        self._transitions = transitions.copy()
         self._state = initial_state
 
     @property

src/ska/base/release.py

@@ -7,7 +7,7 @@
 """Release information for lmc-base-classes Python Package"""
 
 name = """lmcbaseclasses"""
-version = "0.6.1"
+version = "0.6.2"
 version_info = version.split(".")
 description = """A set of generic base devices for SKA Telescope."""
 author = "SKA India and SARAO and CSIRO"

tests/test_control_model.py

+"""Tests for ska.base.control_model."""
+import pytest
+
+from collections import Counter
+
+from ska.base.control_model import DeviceStateModel
+from ska.base.faults import StateModelError
+
+
+class OnOffModel(DeviceStateModel):
+    __transitions = {
+        ("ON", "off"): (
+            "OFF",
+            lambda self: self.count_transition("on->off"),
+        ),
+        ("OFF", "on"): (
+            "ON",
+            lambda self: self.count_transition("off->on"),
+        ),
+    }
+
+    def __init__(self):
+        super().__init__(self.__transitions, "ON")
+        self.counter = Counter()
+
+    def count_transition(self, name):
+        self.counter[name] += 1
+
+
+def test_initial_state():
+    on_off = OnOffModel()
+    assert on_off.state == "ON"
+
+
+def test_try_valid_action_returns_true_and_does_not_change_state():
+    on_off = OnOffModel()
+    assert on_off.try_action("off")
+    assert on_off.state == "ON"
+
+
+def test_try_invalid_action_raises_and_does_not_change_state():
+    on_off = OnOffModel()
+    with pytest.raises(StateModelError):
+        on_off.try_action("on")
+    assert on_off.state == "ON"
+
+
+def test_valid_state_transitions_succeed():
+    on_off = OnOffModel()
+
+    on_off.perform_action("off")
+    assert on_off.state == "OFF"
+    on_off.perform_action("on")
+    assert on_off.state == "ON"
+
+
+def test_invalid_state_transitions_fail():
+    on_off = OnOffModel()
+
+    with pytest.raises(StateModelError):
+        on_off.perform_action("on")
+    on_off.perform_action("off")
+    with pytest.raises(StateModelError):
+        on_off.perform_action("off")
+    with pytest.raises(StateModelError):
+        on_off.perform_action("invalid")
+
+
+def test_side_effect_is_called():
+    on_off = OnOffModel()
+
+    on_off.perform_action("off")
+    on_off.perform_action("on")
+    on_off.perform_action("off")
+
+    assert on_off.counter["on->off"] == 2
+    assert on_off.counter["off->on"] == 1
+
+
+def test_update_transitions_only_applies_to_instances():
+    on_off = OnOffModel()
+
+    on_off_updated = OnOffModel()
+    on_off_updated.update_transitions({("ON", "break"): ("BROKEN", None)})
+
+    assert on_off.is_action_allowed("off")
+    assert not on_off.is_action_allowed("break")
+    assert on_off_updated.is_action_allowed("off")
+    assert on_off_updated.is_action_allowed("break")

tests/test_obs_device.py

@@ -11,9 +11,12 @@
 # Imports
 import re
 import pytest
+
 from tango import DevState
+from tango.test_context import MultiDeviceTestContext
 
 # PROTECTED REGION ID(SKAObsDevice.test_additional_imports) ENABLED START #
+from ska.base import SKABaseDevice, SKAObsDevice
 from ska.base.control_model import (
     AdminMode, ControlMode, HealthState, ObsMode, ObsState, SimulationMode, TestMode
 )
@@ -171,3 +174,19 @@ class TestSKAObsDevice(object):
         # PROTECTED REGION ID(SKAObsDevice.test_testMode) ENABLED START #
         assert tango_context.device.testMode == TestMode.NONE
         # PROTECTED REGION END #    //  SKAObsDevice.test_testMode
+
+
+def test_multiple_devices_in_same_process():
+
+    # The order here is important - base class last, so that we can
+    # test that the subclass isn't breaking anything.
+    devices_info = (
+        {"class": SKAObsDevice, "devices": [{"name": "test/obs/1"}]},
+        {"class": SKABaseDevice, "devices": [{"name": "test/base/1"}]},
+    )
+
+    with MultiDeviceTestContext(devices_info, process=False) as context:
+        proxy1 = context.get_device("test/obs/1")
+        proxy2 = context.get_device("test/base/1")
+        assert proxy1.State() == DevState.OFF
+        assert proxy2.State() == DevState.OFF