Repository navigation
feat: allow aliased enum PVs to map values to underlying fastcs PVs - #417
shihab-dls wants to merge 11 commits into
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 32 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (8)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughEPICS CA aliases accept structured enum mappings for read, write, and command PVs. The IOC validates PV names and puts, and tests cover mapping behavior and alarms. EPICS ID validation now accepts colons. The demo temperature controller marks successful connections. ChangesEPICS CA enum aliases
EPICS controller ID validation
Temperature controller connection state
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant EPICS_CA_Client
participant EpicsCAIOC
participant Controller_Attribute
EPICS_CA_Client->>EpicsCAIOC: Put mapped enum value
EpicsCAIOC->>Controller_Attribute: Convert and apply mapped value
Controller_Attribute-->>EpicsCAIOC: Attribute update
EpicsCAIOC-->>EPICS_CA_Client: Synchronize alias PV value
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The CA alias changes appear mergeable after normal checks; no supported CA workflow was found to depend on the removed PVI endpoint. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 9.23% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 65 functions across 10 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 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❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #417 +/- ##
==========================================
+ Coverage 91.37% 91.68% +0.30%
==========================================
Files 72 72
Lines 2923 3044 +121
==========================================
+ Hits 2671 2791 +120
- Misses 252 253 +1 ☔ View full report in Codecov by Harness. 🚀 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: 2
🤖 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.
Inline comments:
In `@src/fastcs/transports/epics/ca/ioc.py`:
- Around line 40-45: Validate structured alias PV values from EnumMapping.pv
against EPICS_MAX_NAME_LENGTH before IOC record creation, just as _add_alias
validates string aliases. Update the record-creation paths around the affected
alias handling so oversized structured aliases are rejected or skipped
consistently before they are used.
- Around line 483-515: Update _add_write_enum_alias so successful writes through
enum_attr also synchronize the alias record created for alias.pv, while
preventing the update from invoking convert_to_value recursively. Ensure the
alias record reflects the underlying enum value after direct writes to the
original PV, and preserve the existing alias-to-underlying conversion and alarm
handling.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8e74ed54-6c70-4cde-879d-a01bb35df28b
📒 Files selected for processing (8)
src/fastcs/demo/controllers.pysrc/fastcs/demo/schema.jsonsrc/fastcs/transports/epics/ca/ioc.pysrc/fastcs/transports/epics/options.pytests/data/schema.jsontests/example_softioc.pytests/transports/epics/ca/test_softioc.pytests/transports/epics/ca/test_softioc_system.py
614ef18 to
e38ae41
Compare
This branch has not been deployed
2 participants
Summary by CodeRabbit