Skip to content

Conversation

@evvaaaa
Copy link
Contributor

@evvaaaa evvaaaa commented Nov 11, 2024

Changes

Attributes

Mapping

Controllers can now define

    @property
    def additional_attributes(self) -> dict[str, Attribute]:
        """FastCS will look for attributes on the controller, but additional attributes
        are provided by this method."""
        return {}

which is another way to provide attributes without setattr on the controller. The keys are checked to be distinct from the attributes found from the attributes of the controller.

The user can also pass in search_device_for_attributes=False on the controller which will prevent the mapping automatically searching for Attributes, it will only use additional_attributes.

Description

Added a field called description to Attribute, which is used as DESC on the EPICS record.

Initial value

Added an option for initial_value on SignalR/SignalRW. If provided this will be used instead of the default of the _datatype.dtype for the initial value of the attribute.

Changing the datatype

We still want DataType to be frozen, but we also might want to change the value of a field in it - e.g units, corresponding to EGU in the epics backend.

To do this I've made a method called update_datatype which will change the _datatype of the attribute. This alone wouldn't update the corresponding record, so the backend runs set_update_databack_callback. update_datatype will then run this so changing the attribute datatype will update the backend.

Scan Tasks

Errors

Currently, errors in a scan future will be swallowed. Now an error will be printed.

Killing Scan Tasks

We were getting problems for scan tasks not being torn down after the end of tests. Now the backend will stop the tasks on __del__, or running the new stop_scan_tasks method.

DataTypes

Methods

Rather than just using DataType.dtype() we now have a DataType.initial_value property.
We also have a DataType.cast which is used to cast inputted values and validated against metadata fields in the datatype.

Added metadata to the datatype

Added a bunch of optional fields to String, Int, Float which are used for relevant fields e.g HOPR in the epics backend. Int and Float Attributes are validated against their max and min on put.

New Datatypes (PENDING)

Waveform

A waveform to correspond to a single pv without any pva structure beyond regular pvi.

Table

A waveform which is indexable using pvi PVs generated from names of the columns of the numpy structured datatype. These will be added in a new PVXS backend.

Epics Backend

Options

Naming conventions are now passed into the ioc through EpicsIOCOptions, which can be passed into the backend on init.

Handlers

Poll Period

update_period is now optional in fields, designating that the Updater shouldn't be ran periodically. In panda we get all the changes at once in the top level update scan method. We want to then update all the relevant attributes from there.

@evvaaaa evvaaaa self-assigned this Nov 11, 2024
@evvaaaa evvaaaa marked this pull request as draft November 11, 2024 14:21
@evvaaaa evvaaaa force-pushed the panda-conversion-improvements branch from 26042a2 to 3cd53f9 Compare November 11, 2024 15:29
@evvaaaa evvaaaa force-pushed the panda-conversion-improvements branch from 3cd53f9 to f2554b1 Compare November 11, 2024 15:31
@evvaaaa
Copy link
Contributor Author

evvaaaa commented Nov 12, 2024

Scan tasks are not ending correctly in the tests on main:

========================================================== 30 passed in 2.93s ===========================================================
INFO: PVXS QSRV2 is loaded, permitted, and ENABLED.
Task was destroyed but it is pending!
task: <Task pending name='Task-11' coro=<_create_periodic_scan_task..scan_task() done, defined at /home/eva/Code/FastCS/src/fastcs/backend.py:156> wait_for= cb=[_chain_future.._call_set_state() at /usr/lib64/python3.11/asyncio/futures.py:394]>
Task was destroyed but it is pending!
task: <Task pending name='Task-12' coro=<_create_periodic_scan_task..scan_task() done, defined at /home/eva/Code/FastCS/src/fastcs/backend.py:156> wait_for= cb=[_chain_future.._call_set_state() at /usr/lib64/python3.11/asyncio/futures.py:394]>

Edit

This has been handled in the below commit.

Also made it so that scan tasks are explicitly torn down when the
backend is deleted.
@codecov
Copy link

codecov bot commented Nov 12, 2024

Codecov Report

Attention: Patch coverage is 86.06272% with 40 lines in your changes missing coverage. Please review.

Project coverage is 81.65%. Comparing base (bdab4fa) to head (7cb7dd8).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/fastcs/datatypes.py 82.85% 12 Missing ⚠️
src/fastcs/backends/epics/ioc.py 93.33% 8 Missing ⚠️
src/fastcs/backends/epics/util.py 69.56% 7 Missing ⚠️
src/fastcs/attributes.py 70.58% 5 Missing ⚠️
src/fastcs/backend.py 84.00% 4 Missing ⚠️
src/fastcs/backends/epics/backend.py 70.00% 3 Missing ⚠️
src/fastcs/mapping.py 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #72      +/-   ##
==========================================
- Coverage   82.75%   81.65%   -1.11%     
==========================================
  Files          23       23              
  Lines         928     1074     +146     
==========================================
+ Hits          768      877     +109     
- Misses        160      197      +37     

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


🚨 Try these New Features:

For example, updating an attribute to have an `Int` with different units will make the epics backend update `EGU` of the corresponding record.
@GDYendell
Copy link
Contributor

Looks like this will fix #55

@GDYendell
Copy link
Contributor

This is quite a lot. It would be good to split this into smaller PRs. I think some of it can be merged now and then we can continue developing and discussing the rest. It will also help resolve conflicts with the various PRs @marcelldls is working on in smaller chunks.

@evvaaaa
Copy link
Contributor Author

evvaaaa commented Nov 14, 2024

This is quite a lot. It would be good to split this into smaller PRs. I think some of it can be merged now and then we can continue developing and discussing the rest.

I agree, I had a philosophy of putting all the changes together since I've amended them so much. Let me know which features you want now and I can cherry pick them into their own issues/PRs

@evvaaaa evvaaaa requested a review from GDYendell November 18, 2024 10:33
@evvaaaa evvaaaa marked this pull request as ready for review November 18, 2024 10:33
@evvaaaa evvaaaa marked this pull request as draft November 18, 2024 10:33
@evvaaaa
Copy link
Contributor Author

evvaaaa commented Nov 20, 2024

Closing as these have all been separated out into separate PRs.

(We'll keep panda-conversion-branch for now).

@evvaaaa evvaaaa closed this Nov 20, 2024
@GDYendell GDYendell deleted the panda-conversion-improvements branch December 9, 2025 12:09
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.

3 participants