diff --git a/broker/JFJochStateMachine.cpp b/broker/JFJochStateMachine.cpp index 8db2cd1e4..cc65b946f 100644 --- a/broker/JFJochStateMachine.cpp +++ b/broker/JFJochStateMachine.cpp @@ -158,6 +158,18 @@ bool JFJochStateMachine::TakeDarkMaskInternal(std::unique_lock &ul) auto mask_output = services.Stop(); ul.lock(); + // A cancelled collection still hands back a full-size mask - it is built from whatever dark + // frames arrived, and every pixel that saw none reads as good - so the size check below accepts + // it and the sequence goes on to report a calibration it never took as done. The dark mask is a + // single step, so unlike a pedestal sequence there is no later step whose cancel_sequence check + // would catch it. Refuse it the way the check at the top of this function does. + if (cancel_sequence || mask_output.receiver_output.status.cancelled) { + SetState(JFJochState::Inactive, + "Mask sequence cancelled", + BrokerStatus::MessageSeverity::Error); + return false; + } + if (mask_output.receiver_output.dark_mask_result.size() != local_experiment.GetPixelsNum()) { SetState(JFJochState::Error, "Mask not collected properly", BrokerStatus::MessageSeverity::Error); return false; diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 220d8b724..6925d36cf 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -5,6 +5,7 @@ * Re-initialising the broker after a DECTRIS run that never started no longer freezes it with no way to cancel. * Re-initialising the broker disconnects the previous run's stream reader, so a DECTRIS collection no longer starts and then stays empty because half its stream - the start message included - went to a reader nobody was listening to. +* Cancelling a dark-mask calibration abandons it instead of adopting the partial mask and reporting the calibration as done. ### 1.0.0-rc.168 diff --git a/tests/JFJochStateMachineTest.cpp b/tests/JFJochStateMachineTest.cpp index 3b140ceec..8e94f966e 100644 --- a/tests/JFJochStateMachineTest.cpp +++ b/tests/JFJochStateMachineTest.cpp @@ -551,3 +551,58 @@ TEST_CASE("JFJochStateMachine_PreflightRefusesBeforeStarting") { Catch::Matchers::ContainsSubstring("preflight_refused_2468")); REQUIRE_FALSE(pusher.start_called); } + +namespace { + class NullPusher : public ImagePusher { + public: + void StartDataCollection(StartMessage &) override {} + bool EndDataCollection(const EndMessage &) override { return true; } + bool SendImage(const uint8_t *, size_t, int64_t) override { return true; } + bool SendCalibration(const CompressedImage &) override { return true; } + std::string PrintSetup() const override { return "NullPusher"; } + ImagePusherType GetType() const override { return ImagePusherType::Test; } + }; +} + +// Cancelling the dark-mask collection - the window between the receiver being started and Stop() +// returning, which is where a calibration spends its time - must abandon the sequence. The mask +// analysis always hands back a full-size array, every pixel that saw no frame reading as good, so +// the size check the sequence relies on accepts a mask measured on nothing; and the dark mask is a +// single step, so no later cancel_sequence check catches it. The calibration used to end Idle with +// "Calibration sequence done" on a mask it never measured. +TEST_CASE("JFJochStateMachine_DarkMaskCancelIsNotASuccess") { + Logger logger("JFJochStateMachine_DarkMaskCancelIsNotASuccess"); + + DarkMaskSettings mask_settings; + mask_settings.NumberOfFrames(100).MaxCounts(1).MaxFramesWithCounts(5); + + // Nothing listens on the stream address, so no start message ever arrives and the collection + // stays in the window under test until it is cancelled. + DiffractionExperiment experiment(DetDECTRIS(1024, 1024, "Test", "127.0.0.1")); + experiment.ImportDarkMaskSettings(mask_settings); + + AcquisitionDeviceGroup aq_devices; + NullPusher pusher; + JFJochReceiverService receiver_service(aq_devices, logger, pusher); + + JFJochServices services(logger); + services.Receiver(&receiver_service); + + JFJochStateMachine state_machine(experiment, services, logger); + state_machine.AddDetectorSetup(DetDECTRIS(1024, 1024, "Test", "127.0.0.1")); + state_machine.DebugOnly_SetState(JFJochState::Idle); + + REQUIRE_NOTHROW(state_machine.Pedestal()); + + for (int i = 0; i < 400 && state_machine.GetStatus().state != JFJochState::Calibration; i++) + std::this_thread::sleep_for(25ms); + REQUIRE(state_machine.GetStatus().state == JFJochState::Calibration); + + state_machine.Cancel(); + + REQUIRE_NOTHROW(state_machine.WaitTillMeasurementDone(std::chrono::seconds(30))); + auto status = state_machine.GetStatus(); + REQUIRE(status.state != JFJochState::Idle); + REQUIRE(status.message_severity == BrokerStatus::MessageSeverity::Error); + REQUIRE_THAT(status.message.value_or(""), Catch::Matchers::ContainsSubstring("cancelled")); +}