feat: allow aliases to be passed to ca transport#385
Conversation
|
Warning Review limit reached
More reviews will be available in 54 minutes and 39 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds EPICS PV aliasing support to FastCS. A new ChangesEPICS CA Aliases Support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #385 +/- ##
==========================================
+ Coverage 91.20% 91.22% +0.02%
==========================================
Files 72 72
Lines 2875 2882 +7
==========================================
+ Hits 2622 2629 +7
Misses 253 253 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/fastcs/transports/epics/ca/ioc.py (1)
138-154:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
AttrRWignores explicit readback alias keys.Line 138 only looks up the write PV key, then reuses it for both records. A mapping like
PREFIX:MyPv_RBV -> ...is never read forAttrRW, so that config is silently ignored.Suggested fix (preserve fallback behavior, allow explicit RBV aliases)
- alias = aliases.get(f"{pv_prefix}:{pv_name}", None) + write_alias = aliases.get(f"{pv_prefix}:{pv_name}") + read_alias = aliases.get(f"{pv_prefix}:{pv_name}_RBV", write_alias) match attribute: case AttrRW(): @@ _create_and_link_read_pv( - pv_prefix, f"{pv_name}_RBV", attr_name, alias, attribute + pv_prefix, f"{pv_name}_RBV", attr_name, read_alias, attribute ) _create_and_link_write_pv( - pv_prefix, pv_name, attr_name, alias, attribute + pv_prefix, pv_name, attr_name, write_alias, attribute ) case AttrR(): _create_and_link_read_pv( pv_prefix, pv_name, attr_name, - alias, + aliases.get(f"{pv_prefix}:{pv_name}"), attribute, ) case AttrW(): _create_and_link_write_pv( - pv_prefix, pv_name, attr_name, alias, attribute + pv_prefix, pv_name, attr_name, aliases.get(f"{pv_prefix}:{pv_name}"), attribute ) @@ - if alias: - suffix = "_RBV" if pv_name.endswith("_RBV") else "" - record.add_alias(f"{alias}{suffix}") + if alias: + alias_name = ( + f"{alias}_RBV" if pv_name.endswith("_RBV") and not alias.endswith("_RBV") else alias + ) + record.add_alias(alias_name)Also applies to: 187-190
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fastcs/transports/epics/ca/ioc.py` around lines 138 - 154, The AttrRW branch currently only looks up the alias for the write PV and reuses it for the readback, so explicit RBV alias keys (e.g. f"{pv_prefix}:{pv_name}_RBV") are ignored; change the code to separately resolve an RBV alias with fallback to the write alias (e.g. rbv_alias = aliases.get(f"{pv_prefix}:{pv_name}_RBV", alias)) and pass rbv_alias to _create_and_link_read_pv while passing alias to _create_and_link_write_pv; apply the same pattern to the other AttrRW handling block referenced (around lines 187-190) so explicit RBV alias keys are honored but the previous fallback behavior remains.tests/transports/epics/ca/test_softioc.py (1)
628-632:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix stale expected call signature in waveform test (currently breaks CI).
_create_and_link_read_pvnow receives(pv_prefix, pv_name, attr_name, alias, attribute), but this assertion still expects the old 4-arg call.Proposed test fix
create_mock.assert_called_once_with( - DEVICE, "Waveform1d", "waveform_1d", api.attributes["waveform_1d"] + DEVICE, "Waveform1d", "waveform_1d", None, api.attributes["waveform_1d"] )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/transports/epics/ca/test_softioc.py` around lines 628 - 632, The test's expected call signature is stale: _create_and_link_read_pv now takes (pv_prefix, pv_name, attr_name, alias, attribute), but the assertion in tests/transports/epics/ca/test_softioc.py still expects 4 args; update the create_mock.assert_called_once_with invocation used after EpicsCAIOC([api], {}) to include the missing alias argument between the attr_name ("waveform_1d") and the attribute (api.attributes["waveform_1d"]) — use the same alias value the code passes (e.g., the alias string or None) so the assertion matches _create_and_link_read_pv's new signature.
🧹 Nitpick comments (1)
src/fastcs/transports/epics/options.py (1)
36-40: ⚡ Quick winUpdate
EpicsCAOptionsdocstring; it’s now outdated.Line 38 says this options class is currently empty, but Line 44 adds
aliases. This can mislead users reading config docs.Proposed docstring update
class EpicsCAOptions: """Channel-Access-specific options. - Currently empty: present so ``epicsca:`` survives in fastcs.yaml as the - transport discriminator key. Reserved for future CA-specific knobs. + Channel-Access-specific options configured under ``epicsca:`` in fastcs.yaml. + ``aliases`` maps generated PV names to alias base names. """🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fastcs/transports/epics/options.py` around lines 36 - 40, Update the EpicsCAOptions class docstring to reflect that the class is not empty: mention the added "aliases" attribute and briefly describe its purpose (e.g., mapping alternate transport names to this transport) and any intended usage in fastcs.yaml; edit the docstring in the EpicsCAOptions definition so it no longer claims the class is currently empty and instead documents the aliases field and its role for the epicsca transport discriminator.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/fastcs/transports/epics/ca/ioc.py`:
- Around line 138-154: The AttrRW branch currently only looks up the alias for
the write PV and reuses it for the readback, so explicit RBV alias keys (e.g.
f"{pv_prefix}:{pv_name}_RBV") are ignored; change the code to separately resolve
an RBV alias with fallback to the write alias (e.g. rbv_alias =
aliases.get(f"{pv_prefix}:{pv_name}_RBV", alias)) and pass rbv_alias to
_create_and_link_read_pv while passing alias to _create_and_link_write_pv; apply
the same pattern to the other AttrRW handling block referenced (around lines
187-190) so explicit RBV alias keys are honored but the previous fallback
behavior remains.
In `@tests/transports/epics/ca/test_softioc.py`:
- Around line 628-632: The test's expected call signature is stale:
_create_and_link_read_pv now takes (pv_prefix, pv_name, attr_name, alias,
attribute), but the assertion in tests/transports/epics/ca/test_softioc.py still
expects 4 args; update the create_mock.assert_called_once_with invocation used
after EpicsCAIOC([api], {}) to include the missing alias argument between the
attr_name ("waveform_1d") and the attribute (api.attributes["waveform_1d"]) —
use the same alias value the code passes (e.g., the alias string or None) so the
assertion matches _create_and_link_read_pv's new signature.
---
Nitpick comments:
In `@src/fastcs/transports/epics/options.py`:
- Around line 36-40: Update the EpicsCAOptions class docstring to reflect that
the class is not empty: mention the added "aliases" attribute and briefly
describe its purpose (e.g., mapping alternate transport names to this transport)
and any intended usage in fastcs.yaml; edit the docstring in the EpicsCAOptions
definition so it no longer claims the class is currently empty and instead
documents the aliases field and its role for the epicsca transport
discriminator.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2da62478-6221-49d5-8c10-f756644d25ab
📒 Files selected for processing (6)
src/fastcs/demo/schema.jsonsrc/fastcs/transports/epics/ca/ioc.pysrc/fastcs/transports/epics/ca/transport.pysrc/fastcs/transports/epics/options.pytests/data/schema.jsontests/transports/epics/ca/test_softioc.py
1 participant
Closes #384
This PR allows passing aliases to the
catransport. An Example would be:Summary by CodeRabbit
New Features
Tests