Skip to content

Deduplicate validate_ca_id and validate_pva_id #365

@gilesknap

Description

@gilesknap

Spotted while reviewing PR #360 commit-by-commit (slice #355, commit 43956050).

src/fastcs/transports/epics/ca/util.py::validate_ca_id and src/fastcs/transports/epics/pva/util.py::validate_pva_id are byte-for-byte identical except:

  • the regex constant name (_CA_ID_RE vs _PVA_ID_RE) — though both currently compile the same pattern ^[A-Za-z0-9_-]+$
  • the literal "EPICS CA id" vs "EPICS PVA id" in the first error message

The shared 60-char limit was correctly hoisted to transports/epics/util.py::EPICS_MAX_NAME_LENGTH, and pv_prefix_from_path already lives there. The function itself was duplicated.

The commit message rationale is "keep id validation a per-transport concern" — defensible if PVA wanted to diverge from CA later, but currently the structure (walk_api() traversal + length check + error message templates) is structural duplication that's unlikely to need to differ between EPICS variants.

Suggested shape

A shared helper in transports/epics/util.py:

def validate_epics_pv_id(
    controller_api: ControllerAPI,
    *,
    transport_label: str,
    id_re: re.Pattern[str],
) -> None:
    ...

…called by 3-line wrappers in ca/util.py and pva/util.py that pass their own _CA_ID_RE / _PVA_ID_RE and label. The per-transport regex constants stay where they are (so each transport can tighten/loosen its alphabet independently); only the structural skeleton is shared.

Tests in test_ca_util.py and test_pva_util.py already cover both wrappers; no new tests should be needed.

Why now

  • Codifies that the EPICS family shares structural validation, not just constants.
  • Fits the user's "prefer generalising existing patterns" guidance on this codebase.
  • Cheap and reversible if PVA later does diverge — re-inline at that point.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions