HDF5 reader: read variable-length string attributes and datasets

ReadAttrStr and HDF5DataSet::ReadString read every string as fixed-length, sizing the buffer by H5Tget_size. For a variable-length string (what h5py writes for a str, UTF-8) that size is a pointer, so the read put a char* into the buffer and returned garbage. transformation_type then never equalled "rotation", and an EIGER master whose goniometer attributes were written that way (depends_on, transformation_type, units on /entry/sample/transformations) was processed as stills although it holds a valid rotation sweep. All string reads now go through one helper, ReadHDF5String, which handles both fixed- and variable-length strings in either character set; ReadElement<string> already handled the variable-length case and now uses the same helper. Test: JFJochReader_Goniometer_VariableLengthStrings reads the rotation axis from a master with fixed-length ASCII and with variable-length UTF-8 string attributes and depends_on dataset. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SVmAWnzCmRKAXVUCdc4iNi
2026-10-09 21:15:01 +02:00
parent 0457cecafa
commit badd7360ca
3 changed files with 136 additions and 47 deletions
@@ -475,6 +475,97 @@ TEST_CASE("JFJochReader_Goniometer", "[HDF5][Full]") {
    REQUIRE(H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_ALL) == 0);
 }

+// h5py writes str attributes and datasets as variable-length UTF-8 strings; our writer writes
+// fixed-length ASCII. A reader that only understood the second read transformation_type of the
+// first as garbage and processed a third-party rotation sweep as stills.
+TEST_CASE("JFJochReader_Goniometer_VariableLengthStrings", "[HDF5][Full]") {
+    DiffractionExperiment x(DetJF(1));
+
+    x.FilePrefix("test_reader_vlen_str").ImagesPerTrigger(5).OverwriteExistingFiles(true);
+    x.BeamX_pxl(100).BeamY_pxl(200).DetectorDistance_mm(150)
+            .IncidentEnergy_keV(WVL_1A_IN_KEV).PixelSigned(false).BitDepthImage(16)
+            .FrameTime(std::chrono::microseconds(500), std::chrono::microseconds(10));
+    x.Goniometer(GoniometerAxis("omega", 95, 0.1f, Coord(1,0,0),{}));
+
+    RegisterHDF5Filter();
+
+    ScanResultGenerator generator(x);
+    std::vector<uint16_t> image(x.GetPixelsNum(), 0);
+    {
+        StartMessage start_message;
+        x.FillMessage(start_message);
+
+        FileWriter file_set(start_message);
+
+        DataMessage message{};
+        for (int i = 0; i < 5; i++) {
+            message.image = CompressedImage(image, x.GetXPixelsNum(), x.GetYPixelsNum());
+            message.number = i;
+            REQUIRE_NOTHROW(file_set.WriteHDF5(message));
+            REQUIRE_NOTHROW(generator.Add(message));
+        }
+
+        EndMessage end_message;
+        end_message.max_image_number = 5;
+        generator.FillEndMessage(end_message);
+        file_set.WriteHDF5(end_message);
+
+        file_set.Finalize();
+    }
+
+    const bool variable_length = GENERATE(false, true);
+    if (variable_length) {
+        hid_t file = H5Fopen("test_reader_vlen_str_master.h5", H5F_ACC_RDWR, H5P_DEFAULT);
+        REQUIRE(file >= 0);
+        hid_t str_type = H5Tcopy(H5T_C_S1);
+        H5Tset_size(str_type, H5T_VARIABLE);
+        H5Tset_cset(str_type, H5T_CSET_UTF8);
+        hid_t scalar = H5Screate(H5S_SCALAR);
+
+        hid_t omega = H5Dopen2(file, "/entry/sample/transformations/omega", H5P_DEFAULT);
+        REQUIRE(omega >= 0);
+        for (const auto &[name, value] : std::vector<std::pair<std::string, std::string>>{
+                 {"transformation_type", "rotation"}, {"depends_on", "."}, {"units", "deg"}}) {
+            H5Adelete(omega, name.c_str());
+            const char *ptr = value.c_str();
+            hid_t attr = H5Acreate2(omega, name.c_str(), str_type, scalar, H5P_DEFAULT, H5P_DEFAULT);
+            REQUIRE(attr >= 0);
+            REQUIRE(H5Awrite(attr, str_type, &ptr) >= 0);
+            H5Aclose(attr);
+        }
+        H5Dclose(omega);
+
+        H5Ldelete(file, "/entry/sample/depends_on", H5P_DEFAULT);
+        const char *dep = "/entry/sample/transformations/omega";
+        hid_t ds = H5Dcreate2(file, "/entry/sample/depends_on", str_type, scalar,
+                              H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
+        REQUIRE(ds >= 0);
+        REQUIRE(H5Dwrite(ds, str_type, H5S_ALL, H5S_ALL, H5P_DEFAULT, &dep) >= 0);
+        H5Dclose(ds);
+
+        H5Sclose(scalar);
+        H5Tclose(str_type);
+        H5Fclose(file);
+    }
+    {
+        JFJochHDF5Reader reader;
+        reader.ReadFile("test_reader_vlen_str_master.h5");
+        auto dataset = reader.GetDataset();
+
+        REQUIRE(dataset->experiment.GetGoniometer().has_value());
+        CHECK(dataset->experiment.GetGoniometer()->IsScanning());
+        CHECK(dataset->experiment.GetGoniometer()->GetName() == "omega");
+        CHECK(dataset->experiment.GetGoniometer()->GetStart_deg() == Catch::Approx(95.0));
+        CHECK(dataset->experiment.GetGoniometer()->GetIncrement_deg() == Catch::Approx(0.1f).margin(0.00001f));
+        CHECK(dataset->experiment.GetGoniometer()->GetAxis().x == 1);
+        CHECK(dataset->experiment.GetGoniometer()->GetAxis().y == 0);
+        CHECK(dataset->experiment.GetGoniometer()->GetAxis().z == 0);
+    }
+    remove("test_reader_vlen_str_master.h5");
+
+    REQUIRE(H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_ALL) == 0);
+}
+
 // The axis name is free-form in the API, on the wire and in the writer - tests/CBORTest.cpp round
 // trips one literally called "z". The reader used to look only for "omega", so a sweep recorded
 // under any other name came back as stills, with nothing to indicate it. This is that case.