Skip to content

Conversation

@Toreil
Copy link
Collaborator

@Toreil Toreil commented Jul 11, 2025

  • Extra comments were added to try and make the communication between the various buffers and the card easier to understand.
  • There were one or two instances of multiple variables being defined containing the same thing (memory addresses of the buffers) which has been removed.
  • The calculation for the ring_buffer_position was very complicated but in the end a lot of the steps were unnecessary. In the original code it was creating a pointer to a slice of the ring buffer and then getting the memory location of that slice, in the new code it calculates that memory location directly from the location of the ring buffer and the memory offset of the slice.

@Toreil Toreil requested a review from schote July 11, 2025 09:09
@github-actions
Copy link

github-actions bot commented Jul 11, 2025

Coverage

Coverage Report
FileStmtsMissCoverMissing
src/console/interfaces
   acquisition_data.py1202480%127–132, 141–142, 148–149, 152–153, 157–161, 176–177, 188–193, 199, 245
   acquisition_parameter.py871089%91–95, 117, 135, 137, 145, 169
   dimensions.py34974%30, 35, 41–44, 49, 55, 60, 65
src/console/pulseq_interpreter
   sequence_provider.py2564782%91, 93, 95, 97–99, 115, 145, 150, 153–155, 187–190, 218–221, 235–236, 278, 302, 331, 339–342, 350–352, 461, 467, 472–476, 504, 539–546, 556, 622–623
src/console/spcm_control
   abstract_device.py79790%2–149
   acquisition_control.py1481480%3–347
   rx_device.py1801800%2–435
   tx_device.py1801800%2–475
src/console/spcm_control/spcm
   pyspcm.py1431430%3–276
   tools.py54540%3–128
src/console/utilities
   json_encoder.py8362%22–24
   load_config.py27270%2–119
   plot.py17170%2–26
src/console/utilities/sequences/calibration
   fid_tx_adjust.py18194%54
   se_tx_adjust.py25292%56–57
src/console/utilities/sequences/spectrometry
   fid.py15150%2–61
   se_spectrum.py24292%42, 65
src/console/utilities/sequences/tse
   tse_3d.py2438266%109–110, 112–113, 115–116, 123–124, 126–128, 130–133, 136–147, 152–155, 162–173, 190, 212–219, 225, 254, 317–334, 341–342, 407, 410–422, 440, 442, 531–541
TOTAL4899102379% 

Tests Skipped Failures Errors Time
215 0 💤 0 ❌ 0 🔥 33.071s ⏱️

@Toreil Toreil marked this pull request as ready for review July 15, 2025 09:41
Copy link
Owner

@schote schote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The renaming makes sense, but I would like to avoid introducing abbreviations for the variable names and would generally keep them to a minimum. I.e. rather use data_buffer_address than data_buffer_addr. I think num_ is fine, but for channel we could also just use the long version. I think that saves us a large glossary and improves readability. What do you think @Toreil ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants