Skip to content

Conversation

@jsouter
Copy link
Contributor

@jsouter jsouter commented Aug 19, 2025

Closes #74

Works, will work on improving how to configure the size of the image widget.

@codecov
Copy link

codecov bot commented Aug 19, 2025

Codecov Report

❌ Patch coverage is 93.75000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.20%. Comparing base (1763528) to head (010f326).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/pvi/_format/bob.py 92.85% 2 Missing ⚠️
src/pvi/_format/screen.py 90.90% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #157      +/-   ##
==========================================
+ Coverage   91.12%   91.20%   +0.07%     
==========================================
  Files          23       23              
  Lines        1465     1523      +58     
==========================================
+ Hits         1335     1389      +54     
- Misses        130      134       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@GDYendell
Copy link
Member

This currently only converts ImageReads at the root of a screen. I think if you move the conversion into generate_component_formatters it should work in groups as well.

@jsouter
Copy link
Contributor Author

jsouter commented Aug 20, 2025

WIP, there are customisation options like enabling color_bars that I'm not sure if they should belong to the dls.bob template (and parsed by the WidgetFormatter) or should be widget options (so FastCS can set them).

I also changed an inconsistency about whether subscreen buttons have labels in groups or outside of groups, not sure which is preferred.
Looks like it also affects label widgets, so will probably have to fix that

@jsouter
Copy link
Contributor Author

jsouter commented Aug 21, 2025

A:
image
B:
image

@GDYendell
Copy link
Member

GDYendell commented Aug 21, 2025

I wonder can have option B, but with some kind of "open on new screen" icon as the button text? This is the best icon I could find...
image

(D050000L font p)

@coretl ?

@coretl
Copy link
Contributor

coretl commented Aug 21, 2025

I think that icon is good. I have no strong opinion between A and B, but I'd like to see that icon at the end of any button text that launches a new screen

@GDYendell
Copy link
Member

I don't think the label text can be the signal name + the icon on the end because to get that icon you have to set the button label font to one that is all just icons.

For now I suggest we go with B and the duplicated label and make a new Issue to use that icon for all buttons that open sub screens.

@GDYendell
Copy link
Member

I think this is doing what we want now. It would be nice to test with live PVs.

image

@GDYendell
Copy link
Member

I have tested this with fastcs-eiger using the pva transport and it works nicely for the monitor stream. I haven't tested the array trace, we can use CatIO to try this but I am going to merge this for now.

@GDYendell GDYendell marked this pull request as ready for review November 27, 2025 11:17
@GDYendell GDYendell merged commit 6edc548 into main Nov 27, 2025
11 checks passed
@GDYendell GDYendell deleted the image-viewer branch November 27, 2025 11:19
@jsouter
Copy link
Contributor Author

jsouter commented Nov 27, 2025

I have tested this with fastcs-eiger using the pva transport and it works nicely for the monitor stream. I haven't tested the array trace, we can use CatIO to try this but I am going to merge this for now.

Sounds good, I tested this previously with live PVs with a branch of FastCS, will give it a try now

@jsouter
Copy link
Contributor Author

jsouter commented Nov 27, 2025

e.g.
image

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.

Implement image viewer widget for waveform PVs

4 participants