fix(variables): accept byte (BYTE/SINT/USINT) IEC locations; revert to 4.2.9#963
Conversation
Complete the IEC location coverage: add %[QIM]B byte addressing for the 8-bit elementary types (BYTE, SINT, USINT), matching strucpp and the runtime image tables (which back %IB/%QB). %MB (memory byte) is accepted too — strucpp compiles it; a runtime byte_memory buffer will follow. Time/date types (TIME/DATE/TOD/DT) remain unlocated by design. Verified in openplc-web: a BYTE var accepts %MB0; cross-width locations (e.g. %IX on a byte type) are still rejected. Revert APP_VERSION + package.json 4.2.10 -> 4.2.9: this location work is a small fix folded into the 4.2.9 line, not a separate release. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WalkthroughThe application version is changed from 4.2.10 to 4.2.9. PLC address mappings and validation are extended for BYTE-family variable types, with tests covering valid byte locations and invalid cross-width locations. ChangesBYTE location validation
Application version alignment
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/frontend/utils/PLC/address-constants/types.ts`:
- Around line 19-24: Update the comments above the IEC size-class definitions to
distinguish validator support from runtime backing: state that the prefixes are
accepted by validation, while clarifying that %MB is intentionally accepted but
not currently supported by the runtime because byte_memory is absent. Remove the
contradictory claim that all three areas match runtime support.
🪄 Autofix (Beta)
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 032d53de-fa0f-430b-b92e-a3a1e7666ea3
📒 Files selected for processing (5)
package.jsonsrc/frontend/data/constants/app-version.tssrc/frontend/store/__tests__/project-validation-variables.test.tssrc/frontend/store/slices/project/validation/variables.tssrc/frontend/utils/PLC/address-constants/types.ts
| // Each IEC size class accepts all three area prefixes — input (I), output (Q) | ||
| // and memory (M) — matching what strucpp and the runtime image tables support: | ||
| // X = bit (BOOL), B = byte (BYTE/SINT/USINT), W = word (INT/UINT/WORD), | ||
| // D = dword (DINT/UDINT/REAL/DWORD), L = lword (LINT/ULINT/LREAL/LWORD). | ||
| // NOTE: %MB (memory byte) is accepted here even though the runtime does not | ||
| // yet back it (no byte_memory buffer) — a runtime fix is planned separately. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the runtime-support claim in the documentation.
Lines 20 and 23-24 contradict each other: the first says all three areas match runtime support, while the latter says %MB has no runtime byte_memory buffer. Clarify that %MB is intentionally validator-supported but not runtime-backed yet.
Proposed wording
-// and memory (M) — matching what strucpp and the runtime image tables support:
+// and memory (M); strucpp supports all three, but the runtime does not yet
+// provide a byte_memory buffer for %MB:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Each IEC size class accepts all three area prefixes — input (I), output (Q) | |
| // and memory (M) — matching what strucpp and the runtime image tables support: | |
| // X = bit (BOOL), B = byte (BYTE/SINT/USINT), W = word (INT/UINT/WORD), | |
| // D = dword (DINT/UDINT/REAL/DWORD), L = lword (LINT/ULINT/LREAL/LWORD). | |
| // NOTE: %MB (memory byte) is accepted here even though the runtime does not | |
| // yet back it (no byte_memory buffer) — a runtime fix is planned separately. | |
| // Each IEC size class accepts all three area prefixes — input (I), output (Q) | |
| // and memory (M); strucpp supports all three, but the runtime does not yet | |
| // provide a byte_memory buffer for %MB: | |
| // X = bit (BOOL), B = byte (BYTE/SINT/USINT), W = word (INT/UINT/WORD), | |
| // D = dword (DINT/UDINT/REAL/DWORD), L = lword (LINT/ULINT/LREAL/LWORD). | |
| // NOTE: %MB (memory byte) is accepted here even though the runtime does not | |
| // yet back it (no byte_memory buffer) — a runtime fix is planned separately. |
🤖 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/frontend/utils/PLC/address-constants/types.ts` around lines 19 - 24,
Update the comments above the IEC size-class definitions to distinguish
validator support from runtime backing: state that the prefixes are accepted by
validation, while clarifying that %MB is intentionally accepted but not
currently supported by the runtime because byte_memory is absent. Remove the
contradictory claim that all three areas match runtime support.
1 participant
Completes IEC location coverage in the variable-table validator.
Change
BYTE_LOCATION_REGEX = /^%[QIM]B\d+$/and handleBYTE/SINT/USINT→ byte addressing (%IB/%QB/%MB), matching strucpp and the runtime image tables. This closes the last size-class gap (X/B/W/D/L now all supported across I/Q/M).%MB(memory byte) is accepted even though the runtime doesn't back it yet — strucpp compiles it; abyte_memoryruntime buffer will follow separately.Verification
%IB/%QB/%MBaccepted for BYTE/SINT/USINT; cross-width locations still rejected.%MB0.Backed by an end-to-end check: strucpp's lexer/compiler accept all
[IQM][XBWDL]forms, and the runtime image tables back every combination except%MB(tracked for a follow-up runtime fix).🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests
Chores