Add support for BYTE, WORD and DWORD variables - #10
Merged
Conversation
These types were identified correctly by csvGetVarInfo() - PV_ninfo
returns 17/18/19 and the CSV_TYPE_enum values match - but were then
rejected as unsupported in both directions. A DWORD line left the target
variable untouched, incremented FailedLineCount, and surfaced only as a
generic CSV_ERR_LINEFAILURE with the detail buried in the log.
Writing is hex by default, since a bit string is not an ordinary number
and reads better as one. IN.CFG.BitStringFormat selects the notation:
CSV_BITFORMAT_HEX_0X 0x00FF (default)
CSV_BITFORMAT_HEX_IEC 16#00FF
CSV_BITFORMAT_DECIMAL 255
Decimal is there for exchanging files with systems that do not accept
hex - the same reason VarTools formats these types as decimal.
Values are zero padded to the width of the source type (2, 4 or 8
digits) so a column lines up and the width of the underlying variable is
visible in the file.
Reading is permissive regardless of the setting: a prefixed literal
(0xFF, 16#FF, $FF) is hex, bare digits are decimal. Requiring the prefix
for hex is what keeps this unambiguous - otherwise "10" could reasonably
mean either ten or sixteen. Decimal is accumulated by hand rather than
with atoui(), which saturates on overflow instead of reporting it, and
every value is range checked against the width of the target.
LWORD is deliberately still unsupported; it needs 64 bit parsing and a
ULINT store, which CSVFileLib does not have either.
Requires StringExt 1.1.0 for HexStringToUDINT()/UDINTToHexString().
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The example project is built from the committed library copy - there is no lpm install step in build.yml - so the vendored binary has to be refreshed for HexStringToUDINT()/UDINTToHexString() to link. Refreshed from the published @loupeteam/stringext@1.1.0 package, keeping the existing file names so this is a content change rather than a set of case-only renames. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sclaiborne
marked this pull request as ready for review
August 20, 2026 18:51
The example project was a build harness with no variables and no test code, so nothing exercised the new BYTE/WORD/DWORD paths. There was no existing test suite to extend, so this adds one. The suite covers, all verified on ARsim (93 assertions, 0 failures): - PV_ninfo really reports 17/18/19 for BYTE/WORD/DWORD, which is what makes the feature reachable at all. This was the open question from review - it is now measured rather than assumed. - 41 read cases: every accepted prefix, bare decimal, mixed case, zero padding, whitespace, the full 32 bit range from both notations, the per-width limits, and 14 malformed inputs. Rejected values are also checked to leave the target variable untouched. - 12 write cases including the hex versus decimal padding asymmetry and an out of range BitStringFormat. - 36 round trips: 3 formats x 3 widths x 4 values. - End to end through real PVs, and two hand written files that exercise csvParseLine and csvGetVarInfo rather than calling csvOpenVar directly. One of them has an out of range line, to confirm the bad line fails while the good lines still load. The ARsim configuration is what makes the suite runnable; the existing Intel and ARM configurations target hardware. It is pinned to the same AR 6.6.2 as the others and added to CI. Also apply two points left over from review: initialise ValueUdint, which was safe only through short circuit evaluation, and record in the changelog that opening a file now writes bit string variables that were previously left untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The library change itself came back clean; these are all fixes to the suite and the configuration around it. A hang could read as a pass. The documented criterion was "testFail is 0", which stays true if a file operation never completes and the suite parks mid-run. Added suiteOk, which requires no failures, the full expected assertion count, and that the suite actually finished. Verified by removing an input file: suiteOk comes back FALSE with testPass 92 and firstFail naming the phase, and TRUE again once restored. A re-run left stale state. CSVFn_Cyclic walks every VariableList entry regardless of the empty terminator, so the shorter list built for bitstring.csv inherited rows from the longer result.csv list. Both lists are now cleared before being built. Also from review: - Assert that crafted.csv loaded without a line failure, not just that the four values match - Give an empty input its own label so firstFail is never blank - Remove the unreachable state 4 - Note the little endian assumption in the narrow store comparison Configuration hygiene, all inherited from the sample this was derived from rather than intended: - Replace the User.user carrying a BR_Engineer argon2id hash with the Anonymous/Everyone one the sibling configurations use - Name the ConfigurationID AsProject_ARsim, matching Intel and ARM - Write a real Hardware.hwl instead of committing a 0 byte file README rewritten: the partition.json mapping is the Loupe VS Code PLC Toolkit / bnr-build mechanism, not something LPM or the AS build does, so following the old wording left the input files absent. It now also says CI compiles the suite but does not run it, states suiteOk as the verdict, and records that the PV_ninfo codes were measured on AR 6.7.6 while the configurations pin 6.6.2. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The flag was only ever assigned in state 9. The error paths in states 1 and 2 jump straight to state 3 and never reach it, so a re-run whose bitstring.csv save or open failed would keep the previous run's TRUE - a false pass on exactly the axis suiteOk was added to close, and the stale result.csv on the device would agree with it. Clearing on entry rather than recomputing on the way out keeps the file carrying this run's verdict, which was the reason it is computed before the save. Also correct the header comment, which claimed the expression tests testDone. It does not; completeness comes from state 9 being reachable only after the full chain, which is what the corrected wording says. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on StringExt 1.1.0 (StringExt#12, merged and published as
v1.1.0).Problem
BYTE/WORD/DWORDwere identified correctly —PV_ninforeturns 17/18/19 and theCSV_TYPE_enumvalues match — and then rejected as unsupported in bothcsvOpenVarandcsvSaveVar. A DWORD line left the target variable silently untouched, incrementedFailedLineCount, and surfaced only as a genericCSV_ERR_LINEFAILURE, with the actual reason buried in the.csvlog.Writing — hex by default, configurable
New
IN.CFG.BitStringFormat:CSV_BITFORMAT_HEX_0X(0, default)0x00FFCSV_BITFORMAT_HEX_IEC16#00FFCSV_BITFORMAT_DECIMAL255Default 0 means hex without anyone configuring anything, and no
CSVFn_Initchange is needed. Decimal exists for exchanging files with systems that don't accept hex — the same reason VarTools formats these types as decimal, which stays as it is.In hex modes, values are zero-padded to the width of the source type (2/4/8 digits), so a column lines up and the width of the underlying variable is visible in the file. Decimal output is not padded (a BYTE of 5 writes
0x05in hex,5in decimal).Reading — permissive, regardless of the setting
A prefixed literal (
0xFF,16#FF,$FF) is hex; bare digits are decimal. Requiring the prefix for hex is what makes this unambiguous — otherwise10could reasonably mean either ten or sixteen. This means files written in either mode read back, and hand-written files behave the way someone would expect.Two details:
atoui(), which saturates on overflow instead of reporting it — so99999999999is rejected rather than quietly becoming4294967295.0x1FFinto aBYTEis an error rather than a truncation.LWORDis deliberately still unsupported — it needs 64-bit parsing and aULINTstore, which CSVFileLib doesn't have either. That should stand on its own.Compatibility
No file CSVFileLib ever produced contains a bit-string value (they were rejected, and on save the original line was written back verbatim), so there is no legacy format to preserve. Existing files and every other type are unaffected.
Reviewer note
The new cases sit immediately before the "Unsupported and Invalid types" banner in both
csvOpenVarandcsvSaveVar. Placement matters here: inside that group,STRUCT/ULINT/DATE/ARRAY_OF_STRUCTwould fall through into the bit-string handler and write to a struct's address as if it were a DWORD. Worth confirming the fallthrough group is intact when reading the diff.Verification
csvOpenVar.c-Woverflow/-Wmaybe-uninitializedwarnings in the log are pre-existing, in the LREAL path.)The second commit refreshes the vendored
example/.../Loupe/stringextto 1.1.0 along withpackage.jsonandpackage-lock.json— necessary becausebuild.ymlhas nolpm installstep and builds the committed copy. Existing file names were kept so it reads as a content change rather than case-only renames.🤖 Generated with Claude Code