Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Add trigger to planar mode #150

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

WIP: Add trigger to planar mode #150

wants to merge 2 commits into from

Conversation

nvbln
Copy link
Contributor

@nvbln nvbln commented Mar 1, 2024

Adds a trigger to planar mode such that the frame rate can be set explicitly. This is what you would expect given the GUI but not what was actually happening behind the scenes. This solves the third point in #144.

In principle this does away with the non-triggered planar mode, but as I understood it has not been useful so far because there was no reliable way of getting the timing of the frames (see #2).

Since the basic functionality is working (feedback would be great!), I'm creating a PR, but the new class has to be reworked. Currently it is a merge between the old PlanarScanLoop and the VolumetricScanLoop without regard to what is the most maintainable/elegant way of writing. The main reason for this is that I do not fully understand the working of some parts of the code.

self.camera_pulses = RollingBuffer(buffer_len)
set_impulses(
self.camera_pulses.buffer,
1,
Copy link
Contributor Author

@nvbln nvbln Mar 1, 2024

Choose a reason for hiding this comment

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

I'm abusing the set_impulses function here to get a camera pulses buffer for planar mode by setting the number of planes to one. I think we can get rid of this function and instead simply set the first item in the buffer to 5. Increasing the frame rate will be reflected in n_samples_period() becoming shorter, so no further modification of the camera pulses buffer is necessary.

n_samples_trigger = int(
round(self.sample_rate / self.parameters.triggering.frequency)
)
return max(n_samples_trigger, super().n_samples_period())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Admittedly, I don't fully understand what is happening here. In the triggerless planar mode this used to be encapsulated by the least common multiple. I changed it to max() like in the VolumetricScanLoop as it made sense to me that it would be the size of the frame trigger plus the time until the next one.

@nvbln
Copy link
Contributor Author

nvbln commented Mar 21, 2024

I found two problems while testing that should be fixed before merging:

  • You need to add calibration points (meant for volumetric mode) in order to use planar mode.
  • Recording cannot go above 10Hz.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant