Skip to content

Change PrawnBlaster read_status to use readline instead of readlines#146

Merged
dihm merged 2 commits intolabscript-suite:masterfrom
carterturn:carterturn-prawnblaster-status-fix
Apr 7, 2026
Merged

Change PrawnBlaster read_status to use readline instead of readlines#146
dihm merged 2 commits intolabscript-suite:masterfrom
carterturn:carterturn-prawnblaster-status-fix

Conversation

@carterturn
Copy link
Copy Markdown
Contributor

@carterturn carterturn commented Apr 7, 2026

readlines relies on timeouts, so it can be slow. See issue #145 for more details; this PR implements option 1.

@carterturn
Copy link
Copy Markdown
Contributor Author

I have installed this version on my lab's apparatus and confirmed that transition to manual is much faster (no longer noticeable). Experiment behavior seems correct, but it would probably be good to verify that in another environment as well.

Copy link
Copy Markdown
Contributor

@dihm dihm left a comment

Choose a reason for hiding this comment

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

I'd prefer the solution used in the PrawnDO. Basically, use readline, if there is something up, then run readlines to clear out the full buffer and throw the error. A straight copy-paste of the logic used there should work.

@carterturn
Copy link
Copy Markdown
Contributor Author

That is a good idea; I should have checked the PrawnDO code first.
Do you know if there is a reason the PrawnBlaster code has an if case for response? If response was None, I suppose the += might fail, but readline/readlines seem to return empty strings in that case.
The PrawnDO does not have this if case, so I am inclined to remove it here as well.

@dihm
Copy link
Copy Markdown
Contributor

dihm commented Apr 7, 2026

That is a good idea; I should have checked the PrawnDO code first. Do you know if there is a reason the PrawnBlaster code has an if case for response? If response was None, I suppose the += might fail, but readline/readlines seem to return empty strings in that case. The PrawnDO does not have this if case, so I am inclined to remove it here as well.

I don't know why that response check was there. Happy to remove it.

Copy link
Copy Markdown
Contributor

@dihm dihm left a comment

Choose a reason for hiding this comment

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

LGTM

@dihm dihm merged commit 424b9f4 into labscript-suite:master Apr 7, 2026
1 check passed
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