Fix EPICA CA string truncation

[GDYendell]

This accounts for a null terminator at the end of the string in the record.

Summary by CodeRabbit

• Bug Fixes
  • Adjusted string length handling in the Channel Access transport so string fields reserve an extra byte for proper formatting.
• Tests
  • Updated test expectations to reflect the increased string length.

[coderabbitai]

📝 Walkthrough

Walkthrough

String datatype length handling in EPICS CA record creation was adjusted: both input and output record builders now add one to the string length (using attribute.datatype.length + 1 when present, or DEFAULT_STRING_WAVEFORM_LENGTH + 1 when not).

Changes

Cohort / File(s)	Summary
String Length Calculation src/fastcs/transports/epics/ca/util.py	Updated _make_in_record and _make_out_record to use attribute.datatype.length + 1 when a length is present, otherwise DEFAULT_STRING_WAVEFORM_LENGTH + 1 for string record length parameters. Also updated related test expectation.
Tests tests/transports/epics/ca/test_softioc.py	Adjusted test parametrization to expect string length of 257 (previously 256) for read records.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I nibble bytes by moonlight's hum,
One extra hop for strings to come,
A tiny margin, snug and bright,
Records stretch with soft delight,
Hoppity-hop, the fix is done!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Fix EPICA CA string truncation' directly and clearly describes the main change: fixing string truncation in EPICS CA by accounting for null terminators in string records.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch epics-ca-string-truncation

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/fastcs/transports/epics/ca/util.py (1)

247-251: ⚠️ Potential issue | 🟠 Major

Apply the DEFAULT_STRING_WAVEFORM_LENGTH - 1 adjustment to explicit-length strings as well.

The implicit-length case (line 251) correctly accounts for EPICS' NUL terminator by subtracting 1, but the explicit-length case (line 249) does not. In pythonSoftIOC, longStringIn/Out(length=N) sets NELM=N, leaving only N-1 bytes for usable payload. Currently, String(length=3) truncates to 3 characters but the IOC record can only store 2. Change line 249 to return value[: string.length - 1] to keep both paths consistent with the NELM-to-payload semantics.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/fastcs/transports/epics/ca/util.py` around lines 247 - 251, In the String
handling branch (the case matching String() as string), adjust the
explicit-length slicing to account for EPICS NUL terminator semantics the same
way the implicit-length path does: replace the current slice using string.length
with one using string.length - 1 so explicit longStringIn/Out(length=N) yields
at most N-1 usable characters; keep the implicit branch using
DEFAULT_STRING_WAVEFORM_LENGTH - 1 unchanged so both paths use NELM-to-payload
semantics consistently.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/fastcs/transports/epics/ca/util.py`:
- Around line 247-251: In the String handling branch (the case matching String()
as string), adjust the explicit-length slicing to account for EPICS NUL
terminator semantics the same way the implicit-length path does: replace the
current slice using string.length with one using string.length - 1 so explicit
longStringIn/Out(length=N) yields at most N-1 usable characters; keep the
implicit branch using DEFAULT_STRING_WAVEFORM_LENGTH - 1 unchanged so both paths
use NELM-to-payload semantics consistently.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9bd23e02-ebe5-4b15-8a9d-9cc0fd5763a6

📥 Commits

Reviewing files that changed from the base of the PR and between afcbf45 and 5282273.

📒 Files selected for processing (2)

• src/fastcs/transports/epics/ca/util.py
• tests/transports/epics/ca/test_ca_util.py

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/transports/epics/ca/test_softioc.py (1)

69-76: ⚠️ Potential issue | 🟡 Minor

Cover the remaining string-length branches.

Line 75 now checks the default String() path, but this PR’s production change also affects the String(length=...) branch in _make_in_record and the matching _make_out_record path. As-is, either of those can regress without this suite failing.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/transports/epics/ca/test_softioc.py` around lines 69 - 76, Add
parametrize cases to cover the String(length=...) branches so the tests exercise
both _make_in_record and _make_out_record logic for non-default lengths;
specifically add entries using AttrR(String(length=257)) and another for a
shorter length (e.g., 256) with appropriate record_type
("longStringIn"/"longStringOut") and kwargs mirroring the existing case (length,
DESC, initial_value) so the tests assert behavior for the explicit-length path
in _make_in_record and the corresponding _make_out_record handling.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@tests/transports/epics/ca/test_softioc.py`:
- Around line 69-76: Add parametrize cases to cover the String(length=...)
branches so the tests exercise both _make_in_record and _make_out_record logic
for non-default lengths; specifically add entries using
AttrR(String(length=257)) and another for a shorter length (e.g., 256) with
appropriate record_type ("longStringIn"/"longStringOut") and kwargs mirroring
the existing case (length, DESC, initial_value) so the tests assert behavior for
the explicit-length path in _make_in_record and the corresponding
_make_out_record handling.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0d2de550-5757-49c8-afd3-b5bae3de5476

📥 Commits

Reviewing files that changed from the base of the PR and between 76516a2 and ba10651.

📒 Files selected for processing (2)

• src/fastcs/transports/epics/ca/util.py
• tests/transports/epics/ca/test_softioc.py

🚧 Files skipped from review as they are similar to previous changes (1)

• src/fastcs/transports/epics/ca/util.py