Skip to content

Implement SubControllerVector #192

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Conversation

shihab-dls
Copy link
Contributor

fixes #122

This PR introduces a SubControllerVector, which is a mapping of int to SubController, used in pvi grouping.

Copy link

codecov bot commented Aug 15, 2025

Codecov Report

❌ Patch coverage is 81.63265% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.95%. Comparing base (5231029) to head (bd5f50d).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/fastcs/controller.py 74.28% 9 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #192      +/-   ##
==========================================
- Coverage   92.20%   91.95%   -0.26%     
==========================================
  Files          40       40              
  Lines        2065     2112      +47     
==========================================
+ Hits         1904     1942      +38     
- Misses        161      170       +9     

☔ 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.

):
if isinstance(sub_controller, SubControllerVector):
for index, child in sub_controller.children():
child.__sub_controller_vector = name # noqa: SLF001
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to find an alternative to this

@shihab-dls
Copy link
Contributor Author

shihab-dls commented Aug 18, 2025

Maybe it makes more sense to have self._vector_origin and

@dataclass(frozen=True)
class VectorOrigin:
    name: str
    vector: weakref.ReferenceType[SubControllerVector]

Then, we can dereference the parent vector if we want to do something with knowledge of what controllers are in a group, but also just pass the name to pvi since that is all we need for grouping. I'm unsure of any use cases the vector class has other than pvi grouping though, so I'm not sure is this adds any value.

@GDYendell
Copy link
Contributor

GDYendell commented Aug 18, 2025

@shihab-dls I think the trouble of having to keep track of what vector a sub controller is part of would be avoided if SubControllerVector was a subclass of Controller that gets registered directly with its parent, rather it just being a container and registering its children.

Then with a controller hierarchy like

- Odin
 - FP
  - 1
   - HDF
    - FileName 

the path would be ["fp", 1, "hdf"] where integers in the path are treated specially:

  • For PVs, it does Prefix:FP:1:HDF:FileName
  • For the pvi structure it does {"fp": "fp1": {"hdf": {...}}} (because groups are not allowed to start with a number)
  • For GUI groups we could potentially allow names to start with numbers, or just do the same as pvi structure. Probably the latter as otherwise the subscreen for each fp instance would just have "1" as a title...

If this works as I would like, the FP controller vector itself could have attributes and commands. For example here FrameProcessorAdapterController would be a vector with FrameProcessorController elements.

There may be complications with as this, but that is how it looks in my head. Does that make some sense? Happy to pair on this and figure out the details.

@shihab-dls
Copy link
Contributor Author

shihab-dls commented Aug 18, 2025

@shihab-dls I think the trouble of having to keep track of what vector a sub controller is part of would be avoided if SubControllerVector was a subclass of Controller that gets registered directly with its parent, rather it just being a container and registering its children.

Then with a controller hierarchy like

- Odin
 - FP
  - 1
   - HDF
    - FileName 

the path would be ["fp", 1, "hdf"] where integers in the path are treated specially:

* For PVs, it does `Prefix:FP:1:HDF:FileName`

* For the pvi structure it does `{"fp": "fp1": {"hdf": {...}}}` (because groups are not allowed to start with a number)

* For GUI groups we could potentially allow names to start with numbers, or just do the same as pvi structure. Probably the latter as otherwise the subscreen for each fp instance would just have "1" as a title...

If this works as I would like, the FP controller vector itself could have attributes and commands. For example here FrameProcessorAdapterController would be a vector with FrameProcessorController elements.

There may be complications with as this, but that is how it looks in my head. Does that make some sense? Happy to pair on this and figure out the details.

I like this idea (adopting more from ophyds DeviceVector); I had initially started with this line of thinking, but was trying to think of a way to group controllers without relying on their names, given I think we lose information about what controller is or is not a vector by the time we get to structuring the pvi tree. Starting at "A SubControllerVector is a Controller" is probably a more valuable foundation though, so I'll explore some approaches (and probably ask for your input tomorrow :) )

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.

Create ControllerVector to allow making an array of sub controllers of a given class distinguished by an index
2 participants