From 263e74f4de204ecc522ec6f88d782406306e5ee2 Mon Sep 17 00:00:00 2001 From: CamDavidsonPilon Date: Fri, 15 Sep 2023 15:52:51 -0400 Subject: [PATCH] fix #422 --- .../actions/leader/experiment_profile.py | 5 +- .../background_jobs/subjobs/__init__.py | 9 ++++ pioreactor/tests/test_temperature_control.py | 49 +++++++++++++------ 3 files changed, 46 insertions(+), 17 deletions(-) diff --git a/pioreactor/actions/leader/experiment_profile.py b/pioreactor/actions/leader/experiment_profile.py index f1519ed1..b5d1cb7e 100644 --- a/pioreactor/actions/leader/experiment_profile.py +++ b/pioreactor/actions/leader/experiment_profile.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- from __future__ import annotations +from pathlib import Path from threading import Timer from typing import Callable @@ -176,11 +177,11 @@ def execute_experiment_profile(profile_filename: str, dry_run: bool = False) -> if dry_run: logger.notice( # type: ignore - f"Executing DRY-RUN of profile {profile.experiment_profile_name}, sourced from {profile_filename}. See logs." + f"Executing DRY-RUN of profile {profile.experiment_profile_name}, sourced from {Path(profile_filename).name}. See logs." ) else: logger.notice( # type: ignore - f"Executing profile {profile.experiment_profile_name}, sourced from {profile_filename}." + f"Executing profile {profile.experiment_profile_name}, sourced from {Path(profile_filename).name}." ) try: diff --git a/pioreactor/background_jobs/subjobs/__init__.py b/pioreactor/background_jobs/subjobs/__init__.py index f399317d..11012668 100644 --- a/pioreactor/background_jobs/subjobs/__init__.py +++ b/pioreactor/background_jobs/subjobs/__init__.py @@ -7,6 +7,15 @@ class BackgroundSubJob(BackgroundJob): # don't listen for signal handlers - parents take care of disconnecting us. But we _must_ be a child. + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + # don't allow state changes to occur over MQTT. Again, only the parent should control the childs state. + # use `controller.set_state("sleeping")` or `controller.automation_job.set_state("sleeping")` (prefer the former.) + state_settings = self.published_settings["state"] + state_settings["settable"] = False + self.add_to_published_settings("state", state_settings) + def _set_up_exit_protocol(self): # NOOP: subjobs don't exit from python, parents do. return diff --git a/pioreactor/tests/test_temperature_control.py b/pioreactor/tests/test_temperature_control.py index 7650a070..8a965682 100644 --- a/pioreactor/tests/test_temperature_control.py +++ b/pioreactor/tests/test_temperature_control.py @@ -25,7 +25,7 @@ def pause(n=1) -> None: def test_thermostat_automation() -> None: experiment = "test_thermostat_automation" with temperature_control.TemperatureController( - "thermostat", target_temperature=50, unit=unit, experiment=experiment + automation_name="thermostat", target_temperature=50, unit=unit, experiment=experiment ) as algo: pause(2) @@ -56,7 +56,7 @@ def test_changing_temperature_algo_over_mqtt() -> None: ) with temperature_control.TemperatureController( - "only_record_temperature", unit=unit, experiment=experiment + unit=unit, experiment=experiment, automation_name="only_record_temperature" ) as tc: assert tc.automation_name == "only_record_temperature" assert isinstance(tc.automation_job, OnlyRecordTemperature) @@ -86,7 +86,7 @@ def test_changing_temperature_algo_over_mqtt_and_then_update_params() -> None: ) with temperature_control.TemperatureController( - "only_record_temperature", unit=unit, experiment=experiment + unit=unit, experiment=experiment, automation_name="only_record_temperature" ) as algo: assert algo.automation_name == "only_record_temperature" assert isinstance(algo.automation_job, OnlyRecordTemperature) @@ -113,7 +113,7 @@ def test_changing_temperature_algo_over_mqtt_and_then_update_params() -> None: def test_heating_is_reduced_when_set_temp_is_exceeded() -> None: experiment = "test_heating_is_reduced_when_set_temp_is_exceeded" with temperature_control.TemperatureController( - "only_record_temperature", unit=unit, experiment=experiment + unit=unit, experiment=experiment, automation_name="only_record_temperature" ) as t: setattr( t.heating_pcb_tmp_driver, @@ -134,7 +134,7 @@ def test_heating_is_reduced_when_set_temp_is_exceeded() -> None: def test_thermostat_doesnt_fail_when_initial_target_is_less_than_initial_temperature() -> None: experiment = "test_thermostat_doesnt_fail_when_initial_target_is_less_than_initial_temperature" with temperature_control.TemperatureController( - "thermostat", unit=unit, experiment=experiment, target_temperature=20 + unit=unit, experiment=experiment, automation_name="thermostat", target_temperature=20 ) as t: pause(3) assert t.automation_job.state == "ready" @@ -144,7 +144,7 @@ def test_thermostat_doesnt_fail_when_initial_target_is_less_than_initial_tempera def test_heating_stops_when_max_temp_is_exceeded() -> None: experiment = "test_heating_stops_when_max_temp_is_exceeded" with temperature_control.TemperatureController( - "thermostat", + automation_name="thermostat", unit=unit, experiment=experiment, target_temperature=25, @@ -172,10 +172,9 @@ def test_heating_stops_when_max_temp_is_exceeded() -> None: def test_child_cant_update_heater_when_locked() -> None: experiment = "test_child_cant_update_heater_when_locked" with temperature_control.TemperatureController( - "only_record_temperature", unit=unit, experiment=experiment, - eval_and_publish_immediately=False, + automation_name="only_record_temperature", ) as t: assert t.automation_job.update_heater(50) @@ -196,7 +195,7 @@ def test_constant_duty_cycle_init() -> None: dc = 50 with temperature_control.TemperatureController( - "constant_duty_cycle", unit=unit, experiment=experiment, duty_cycle=dc + automation_name="constant_duty_cycle", unit=unit, experiment=experiment, duty_cycle=dc ) as algo: pause() assert algo.heater_duty_cycle == 50 @@ -206,7 +205,7 @@ def test_setting_pid_control_after_startup_will_start_some_heating() -> None: # this test tries to replicate what a user does in the UI experiment = "test_setting_pid_control_after_startup_will_start_some_heating" with temperature_control.TemperatureController( - "thermostat", unit=unit, experiment=experiment, target_temperature=35 + automation_name="thermostat", unit=unit, experiment=experiment, target_temperature=35 ) as t: pause(3) assert t.automation_job.state == "ready" @@ -226,7 +225,7 @@ def collect(msg) -> None: ) with temperature_control.TemperatureController( - "only_record_temperature", unit=unit, experiment=experiment + automation_name="only_record_temperature", unit=unit, experiment=experiment ): # change to PID thermostat @@ -255,7 +254,7 @@ def collect(msg) -> None: def test_temperature_control_and_thermostats_relationship() -> None: experiment = "test_temperature_control_and_thermostats_relationship" with temperature_control.TemperatureController( - "thermostat", unit=unit, experiment=experiment, target_temperature=30 + automation_name="thermostat", unit=unit, experiment=experiment, target_temperature=30 ) as tc: tc.publish_temperature_timer.pause() # pause this for now. we will manually run evaluate_and_publish_temperature pause() @@ -303,7 +302,7 @@ def coprime2(a, b): experiment = "test_coprime" with temperature_control.TemperatureController( - "thermostat", unit=unit, experiment=experiment, target_temperature=30 + automation_name="thermostat", unit=unit, experiment=experiment, target_temperature=30 ) as tc: assert coprime2( tc.read_external_temperature_timer.interval, @@ -324,7 +323,7 @@ def execute(self): experiment = "test_using_external_thermocouple" with temperature_control.TemperatureController( - "only_record_temperature", + automation_name="only_record_temperature", unit=unit, experiment=experiment, using_third_party_thermocouple=True, @@ -369,7 +368,7 @@ def test_that_if_a_user_tries_to_change_thermostat_X_to_thermostat_Y_we_just_cha experiment = "test_that_if_a_user_tries_to_change_thermostat_X_to_thermostat_Y_we_just_change_the_attr_instead_of_the_entire_automation" with temperature_control.TemperatureController( - "thermostat", target_temperature=30, unit=unit, experiment=experiment + automation_name="thermostat", target_temperature=30, unit=unit, experiment=experiment ) as tc: assert tc.automation_name == "thermostat" assert isinstance(tc.automation_job, Thermostat) @@ -394,3 +393,23 @@ def test_that_if_a_user_tries_to_change_thermostat_X_to_thermostat_Y_we_just_cha assert hasattr(tc.automation_job, "test_attr") assert tc.automation_job.test_attr + + +def test_that_you_cant_disconnect_the_temperature_automation_directly_but_must_use_the_controller_to_disconnect() -> ( + None +): + experiment = "test_that_you_cant_disconnect_the_temperature_automation_directly_but_must_use_the_controller_to_disconnect" + + with temperature_control.TemperatureController( + unit, experiment, automation_name="thermostat", target_temperature=20 + ) as algo: + assert algo.automation_name == "thermostat" + assert algo.automation_job.state == algo.READY + pause() + pause() + pubsub.publish( + f"pioreactor/{unit}/{experiment}/temperature_automation/$state/set", "disconnected" + ) + pause() + assert algo.automation_job.state == algo.READY + pause()