broker: a cancelled dark-mask collection is not a finished calibration
DarkMaskAnalysis::GetMask always returns a full-size array - a pixel that saw no frame reads as good - so the dark_mask_result.size() check in TakeDarkMaskInternal accepted a mask measured on nothing. The mask was adopted through LoadDarkBadPixelMask, the state went to Idle and the sequence logged "Calibration sequence done" on a calibration the operator had just cancelled. The dark mask is a single step, so unlike a pedestal sequence there is no later cancel_sequence check to catch it. Refuse it after Stop() the way the check at the top of the same function does: Inactive with the "Mask sequence cancelled" error, matching what the pedestal steps report. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012bew392LTGP2fkhfRJsMcB
This commit is contained in:
1 parent
c40e0eb11d
commit
1da94bdfe2
3 files changed
+68
No files matched your search
@@ -158,6 +158,18 @@ bool JFJochStateMachine::TakeDarkMaskInternal(std::unique_lock<std::mutex> &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;
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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"));
|
||||
}
|
||||
Reference in new issue
Block a user