mxcube / mxcubecore

Backend used by MXCuBE
http://mxcube.github.io/mxcube/
GNU Lesser General Public License v3.0
11 stars 51 forks source link

MASSIF-1 Crystal Direct Harvester implementation #826

Closed jbflo closed 2 months ago

jbflo commented 7 months ago

This code related to the implementation of the Crystal Direct Harvester in Massif-1 ESRF

This should not have breaking change. Beamline who does not have an Harvester should not have any new things/modification to do A PR in Mxcube-web related to this one need to be reviewed as well .

github-actions[bot] commented 7 months ago

Coverage

Coverage Report •
FileStmtsMissCoverMissing
mxcubecore/Command/exporter
   ExporterClient.py1138128%53–60, 67–76, 83–92, 99–101, 110–118, 129–137, 145–149, 157–161, 170–177, 186–187, 196–202, 211–218
mxcubecore/HardwareObjects
   Beamline.py32411166%146–160, 261–268, 300, 322, 366, 377, 399, 455, 467, 478, 493, 504, 515, 537, 548, 561, 572, 583, 594, 605, 616, 627, 638, 649, 661, 676, 687, 700, 711, 726, 737, 748, 764, 780, 797–883, 890–900, 903, 906–917
   EMBLFlexHCD.py4604600%21–829
   Harvester.py2342340%21–643
   HarvesterMaintenance.py53530%21–160
   ISPyBRestClient.py2472470%4–608
   Microdiff.py3153150%1–816
   Session.py1747159%84, 98–99, 113, 149–176, 187, 195, 202, 209, 222–228, 241, 263, 279–299, 312–326, 329–336, 344–352, 367–376, 383, 390, 397, 404, 411
mxcubecore/HardwareObjects/abstract
   AbstractSampleChanger.py35315855%249, 252, 261–262, 274–275, 279, 291–292, 301, 309, 316, 323, 338–340, 347, 366–367, 394, 403, 409–411, 418–421, 437–440, 446–453, 479, 483, 487, 491, 498, 516–525, 541–546, 559–564, 580–585, 599–601, 616–628, 646–651, 665, 671, 674–679, 688, 692, 696, 700, 704, 708, 712, 716, 721–733, 748–764, 771–776, 797–812, 815–818, 821–824, 838, 844, 847
mxcubecore/HardwareObjects/abstract/sample_changer
   Crims.py1071070%1–167
mxcubecore/HardwareObjects/mockup
   HarvesterMockup.py1431430%21–465
mxcubecore/model
   queue_model_objects.py1454115221%37–38, 52–63, 67, 73, 83, 90, 97, 108–111, 123, 130, 141–146, 149–157, 160, 163–170, 173–180, 183, 186, 189, 192, 195, 198, 201, 204, 208, 211, 214, 217, 220–228, 231–232, 235–237, 242–246, 249, 256–258, 263–265, 270–273, 276–277, 282–309, 312–313, 316, 319–324, 327–332, 335, 338–343, 346–353, 356–473, 476–496, 499–509, 519–523, 527, 530–545, 548, 551, 554, 557, 560, 563, 566–573, 607–636, 640, 644–648, 676–678, 681, 684, 687, 690, 693, 699, 702, 705, 708, 711, 714–717, 720–723, 728, 731–732, 735–748, 756–757, 765–766, 774, 782–804, 807–808, 811, 814, 819, 824–837, 840, 859–878, 882, 885, 888, 893, 896, 899–903, 906, 913, 918–921, 929–930, 938–944, 947, 956–997, 1000, 1036–1038, 1041–1043, 1048–1065, 1068, 1071, 1074, 1077, 1080, 1083, 1086, 1089, 1092–1093, 1096–1107, 1110–1117, 1120, 1125–1144, 1153–1170, 1173, 1176, 1179, 1182–1183, 1186–1192, 1195, 1198, 1201, 1204, 1207, 1210–1217, 1220, 1225–1228, 1233–1262, 1265, 1268, 1271–1273, 1276, 1295–1305, 1308, 1311, 1314, 1317, 1320, 1323, 1326–1329, 1342–1364, 1383–1398, 1401, 1404, 1407, 1410, 1413, 1416, 1419, 1422–1425, 1435–1444, 1447, 1450, 1455–1458, 1467–1481, 1507–1521, 1524–1540, 1543, 1560–1562, 1565–1576, 1579–1580, 1583–1584, 1594–1651, 1654–1661, 1664–1675, 1678–1685, 1688, 1691–1704, 1707, 1712–1752, 1755–1760, 1763, 1802, 1807–1826, 1829, 1832, 1850–1862, 1865–1867, 1910–1912, 1915–1919, 1922, 1925–1939, 1942, 1945, 1948, 1951, 1954, 1957, 1962–1966, 1969, 1972, 1975, 1980–2049, 2053–2084, 2087–2089, 2121–2217, 2249–2291, 2299–2427, 2432, 2437, 2442, 2447, 2452, 2457, 2460, 2465, 2469–2474, 2479–2484, 2500–2515, 2533–2536, 2546–2565, 2568, 2574, 2577, 2582–2585, 2629–2710, 2729–2736, 2767–2775, 2803–2849, 2853–2856
mxcubecore/queue_entry
   base_queue_entry.py4324320%19–913
TOTAL61198567227% 

Tests Skipped Failures Errors Time
1925 0 :zzz: 0 :x: 0 :fire: 1m 37s :stopwatch:
beteva commented 6 months ago

Not quite @rhfogh, in this case it refers that the flex is manifactured by EMBL, just as Cats, Marvin ...:-)

jbflo commented 3 months ago

Surely a file called EMBLFlexHCD.py should be in HardwareObjects/EMBL, and not in HardwareObjects/ ?

@rhfogh do you still request this change ?

marcus-oscarsson commented 3 months ago

Looks good as far as I can see :), (its a big one ;) )

beteva commented 3 months ago

Hi @jbflo, Most of the changes are Ok with me. It still bothers me, though, that you have to check for harvester in the EMBLFlexHCD.py.

jbflo commented 3 months ago

Hi @jbflo, Most of the changes are Ok with me. It still bothers me, though, that you have to check for harvester in the EMBLFlexHCD.py.

as we We check because depending of where the SC take the Sample , the Sample List and the Next Actions are different

maybe it's because the Dewar is (consider) a part of the FLEX.

Now we should probably think of 2 things. 1- The Sample changer ( the Robot ) FLEX 2 The Sample Holder (Can be a Dewar(HCD) or Harvester)

Or maybe we should have a Flex and a HCD Class ? Or an EMBLFlexHavester CLass ? having them both implementing as one is not much help understanding the Harvester.

Yeah maybe an EMBLFlexHavester CLass but this will have must of the EMBLFlexHCD methode In.

I am thinking about it @beteva , but for now i don't have a better clue. for now I Think this class (EMBLFlexHCD) is the best place.

please let me know if you are thinking of a better logic.

marcus-oscarsson commented 3 months ago

I agree with @beteva that it would be nice if the sample changer and the Harvester code could be de-coupled. I think that would be a nice followup PR.

beteva commented 3 months ago

I am thinking about it @beteva , but for now i don't have a better clue. for now I Think this class (EMBLFlexHCD) is the best place.

please let me know if you are thinking of a better logic.

@jbflo, it might be tempting to split Robot and Holder, but I am afraid that this will add an extra complication to already complicated device. I would rather go for the other idea - having a specific EMBLFlexHavester CLass, which inherits from the EMBLFlexHCD and only overloads the methods which differ for the harvester. Than in the beamlise-setup.yml it will be easy to choose which "sample changer" you have at given moment.What do you think?

jbflo commented 3 months ago

I am thinking about it @beteva , but for now i don't have a better clue. for now I Think this class (EMBLFlexHCD) is the best place. please let me know if you are thinking of a better logic.

@jbflo, it might be tempting to split Robot and Holder, but I am afraid that this will add an extra complication to already complicated device. I would rather go for the other idea - having a specific EMBLFlexHavester CLass, which inherits from the EMBLFlexHCD and only overloads the methods which differ for the harvester. Than in the beamlise-setup.yml it will be easy to choose which "sample changer" you have at given moment.What do you think?

Yes I also think it's a better Idea , I'll go on that side. Thanks !