Skip to content

Conversation

@timholy
Copy link
Member

@timholy timholy commented Feb 23, 2021

This fixes a bug reported via email by @jona125. The key change is allowing the user to pass vertical_lines to write_commands to communicate the anticipated frame size. I am not sure whether there's any way of using the same value to control the acquisition.

I'm happy to accept feedback on any of this, including the naming of the parameter vertical_lines.

If you're acquiring less than the full chip, the frame rate might
exceed what's possible when using the full chip. Allow users
to pass the number of vertical lines acquired as part of write_commands.
@kdw503
Copy link
Member

kdw503 commented Feb 23, 2021

Opps! I already did this and about to do pull request. But your commit looks better.

@timholy
Copy link
Member Author

timholy commented Feb 23, 2021

So sorry! I should have coordinated with you.

@timholy
Copy link
Member Author

timholy commented Feb 23, 2021

Anything that you like better about yours, feel free to add to this branch.

@kdw503
Copy link
Member

kdw503 commented Feb 23, 2021

Other than this topic, If user can use ImaginePlots.jl with ImagineInterface it would be better to work. But ImaginPlots uses UnitfulPlots.jl which is still living in julia0.7 so I forked it to my repo. and added Project.toml then it still works. Is it ok to fork the UnitfulPlots.jl to our Holylab repo. and do the pull request of the modificaitons in ImaginePlots.jl?

@timholy
Copy link
Member Author

timholy commented Feb 23, 2021

Sure. But have you submitted a PR to update the official UnitfulPlots.jl? I don't see any open pull requests.

@kdw503
Copy link
Member

kdw503 commented Feb 23, 2021

I didn't do yet.

@timholy
Copy link
Member Author

timholy commented Feb 23, 2021

Go for it! The world is your oyster, don't be shy. You're good at this stuff! 😃

@kdw503 kdw503 merged commit c4b4056 into master Feb 24, 2021
@kdw503 kdw503 deleted the teh/reduced_framesize branch February 24, 2021 03:15
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.

2 participants