fix(baremetal): bound every I/O slot write, and stop truncating Modbus sizes

[JulioSergioFS]

Pull request info

References

Refs #296 — these are the defects that issue surfaced. The process-image sizing itself is not here (see Scope below).

Paired mirror PR: Autonomy-Logic/openplc-web#729 — merge together, ci-sync needs both.

Supersedes the firmware half of #1069, which is being closed.

Scope

#296 asked why the P1AM-200 has the same I/O ceiling as the P1AM-100, given it has 3x the memory.

The first attempt (now closed: #1069, openplc-packages#46) let each device declare its own image size in its VPP manifest. Review set that aside, and the reason is worth recording: the board's memory is for everything — the user's program, its variables, every buffer — not just I/O. Reserving 3x more image on the -200 eats memory the program needs, and most projects never use that much I/O. It also means guessing a number per board, forever, for boards nobody can measure.

The decision is that the image is sized by the user's program instead: if the program declares 200 I/Os, 200 are allocated. That is not malloc — the compiler knows the size at build time, so on bare metal it becomes a generated #define. The same fixed-image problem exists in Runtime v4 and is being addressed there too. That work belongs to the Modbus Server unification (DOPE-370 / DOPE-371) and is not in this PR.

This PR carries only the part that is a bug regardless of how the image is sized, and that is worth fixing on its own.

Description of the changes proposed

Out-of-bounds writes when binding located variables. runtime_bind_located_vars() wrote bool_input / bool_output / int_* / *_memory at whatever byte_index the descriptor carried. Only the DWord cases were bounded. %QX7.0 against the 56-output image indexes bool_output[7][8] — one past the end — and corrupts whatever follows it. The editor allocates and compiles that address without complaint, so nothing upstream stops it either.

Every slot write is now range-checked. An out-of-range descriptor is skipped, leaving the slot NULL, which every HAL and the Modbus glue already treat as "not wired".

Modbus bank sizes truncated to 8 bits. init_mbregs() took its six sizes as uint8_t and MBinfo stored them the same way, so any bank above 255 wrapped silently — 256 coils became 0, and the register map came up wrong with no diagnostic. Widened to uint16_t, along with byte_addr in get_discrete/write_discrete (addr/8 overflows a uint8_t past 2040 coils) and the pos index into dint_memory/lint_memory.

readCoils aliased every coil above 255. It called get_discrete((uint8_t)startreg) on a 16-bit coil address, so FC 0x01 answered with the wrong bit for anything past 255 — silently, no exception. readInputStatus never had the cast. Harmless while no board had more than 56 coils; a bug the moment one does.

mapEmptyBuffers() no longer allocates per point. It called malloc(1) once per unbound discrete point. PLC firmware avoids the heap on principle — allocation there fragments and never recovers — and one static array costs exactly MAX_DIGITAL_INPUT + MAX_DIGITAL_OUTPUT bytes with no allocator involved.

Note for whoever picks up the sizing work

The openplc.h change that makes MAX_* overridable (#ifndef guards + #include "defines.h") is deliberately not here — it is the enabling mechanism for the sizer, which is someone else's task now. It exists ready to reuse on the branch feature/gh-296-gh-565-process-image-and-located-arrays.

DOD checklist

• The code is complete and according to developers' standards.
• I have performed a self-review of my code.
• Meet the acceptance criteria.
• Unit tests are written and green — the repo has no automated firmware tests; these are C/C++ sources Jest does not reach.
• Test coverage: unchanged (no TypeScript touched).
• Integration tests are written and green.
• Changes were communicated and updated in the ticket description.
• Reviewed and accepted by the Product Owner.
• End-to-end test are successful.

Verification

• tsc --noEmit — 0 errors; jest src/backend/shared/compile green (nothing in TypeScript changed, run as a regression guard).
• compare-surfaces.py against web#729 — match: True, 0 diffs.

Not verified — needs hardware

No board was flashed. Two things want a real check before this is trusted in the field:

1. Build for a board and confirm the guards cost nothing measurable in the scan loop.
2. With more than 255 coils configured, read them from a Modbus master and confirm FC 0x01 answers the right bits — that is the truncating-cast path, which previously failed silently.

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (5)

• resources/sources/Baremetal/Baremetal.ino is excluded by !resources/**
• resources/sources/Baremetal/modbus_registers.cpp is excluded by !resources/**
• resources/sources/Baremetal/modbus_registers.h is excluded by !resources/**
• resources/sources/Baremetal/modbus_types.h is excluded by !resources/**
• resources/sources/arduino/arduino_runtime_glue.cpp is excluded by !resources/**

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 02c73c32-7f76-4ee4-91c6-0e06b00e1991

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

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.}