Skip to content

Fix device access list#282

Merged
JeanLucPons merged 2 commits into
mainfrom
fix-device-access-list
Jun 12, 2026
Merged

Fix device access list#282
JeanLucPons merged 2 commits into
mainfrom
fix-device-access-list

Conversation

@JeanLucPons

@JeanLucPons JeanLucPons commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

This PR fix wrong implementation of DeviceAccessList which was not performing well abstracted method checking.
This PR requires pyaml-cs-oa fix that triggered the issue.
@gupichon I would like that you check carrefully the check the range checking which was not in unit test.
I cannot check myself while tango backend was not update.
Thanks

@JeanLucPons JeanLucPons requested a review from gupichon June 11, 2026 11:37
@gupichon

Copy link
Copy Markdown
Contributor

@JeanLucPons, I will look into it. A unit test exists for ranges but it's a bit basic. (test_ranges_array.py)

@JeanLucPons

Copy link
Copy Markdown
Contributor Author

OK thanks.
In fact it is more difficult than it looks to implements correctly abstract list.

@JeanLucPons JeanLucPons marked this pull request as draft June 11, 2026 12:12
gupichon
gupichon previously approved these changes Jun 11, 2026
@JeanLucPons

Copy link
Copy Markdown
Contributor Author

In fact I will have to remove the fact that DeviceAccessList is a list.
Otherwise the aggregator will have to be fully reconstructed each time a device is added.

@gupichon

Copy link
Copy Markdown
Contributor

Yes, but implementing the sequence the way you did should correct this, thanks to the delegation to the inner list, right?

@JeanLucPons

Copy link
Copy Markdown
Contributor Author

In fact it does not work because the Aggregator has to update its internal structure when an item is added or removed.
Or to reconstruct its internal structure at each list update.
Otherwise classic list behavior cannot be handled.
Today it is working because of the add_devices method but if you use the list normally, it fails.

@JeanLucPons

Copy link
Copy Markdown
Contributor Author

This time it should be OK.
The list behavior of DeviceAccessList has been removed due to potential complex internal struture of aggregator.
Only relevant method are present and the internal structure is not exposed. No longer risk to use the list if a bad way.

@JeanLucPons JeanLucPons requested a review from gupichon June 11, 2026 13:34
@JeanLucPons JeanLucPons marked this pull request as ready for review June 11, 2026 13:40
@JeanLucPons JeanLucPons merged commit 4d606fa into main Jun 12, 2026
3 checks passed
@JeanLucPons JeanLucPons deleted the fix-device-access-list branch June 12, 2026 06: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.

2 participants