merge: DOPE-442 into the S7Comm branch — unlocks servers on baremetal targets - #1109
Merged
Merged
Conversation
`TargetCapabilities` answers "is this available", and stays a flat matrix of
booleans. The unified server screen needs a shapier answer -- which transports,
which IEC segments, which fields are the user's to set -- so that lives in its
own resolver rather than growing the matrix five booleans that only mean
something together.
The differences it encodes are hardware, not preference. A baremetal target:
* has no %MX segment. Baremetal.ino calls init_mbregs with
MAX_DIGITAL_OUTPUT as the coil count and arduino/openplc.h declares no
bool_memory, so a %MX row would name storage that does not exist.
* listens on 502, hard-coded in three places in modbus_tcp.cpp, so a port
input would be a control that changes nothing.
* has buffer sizes fixed at compile time by the MCU's MAX_* constants, which
also size the IEC pointer arrays -- not a Modbus setting at all.
A board whose VPP carries a Modbus screen resolves to the vendor-screen store
even when its capabilities also report modbusTcpServer. That screen IS where
the board's Modbus state already lives; reading a PLCServer instead would
present every existing project as though it had never been configured.
modbusRtuServer joins the matrix because the screen offers a transport
selector, and "which transports" is exactly what it has to ask the target.
arduino-cli reports both: the firmware has always served both, and the flags
read false until now only because the config sat in a screen the Servers UX
could not see.
BoardInfo gains `io`, the firmware buffer sizes a VPP declares per device. The
editor cannot derive them -- they are chosen per MCU family inside a header the
build never reports back. A partial block resolves to no counts at all rather
than being filled with zeros, which would push every later segment onto the
wrong Modbus offset.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Modbus server screen now renders from the target's profile instead of assuming Runtime v4. A baremetal board gets a transport selector (RTU, TCP or both), its slave id, a read-only port and a read-only address map; a Runtime v4 target gets what it always had. The two stores stay where they are, behind `useModbusServerConfig`. A PLCServer is a project element; a board's Modbus config is board state in `vendorScreenData`, archived per board. Merging them would have cost a project migration, a rewrite of the defines emitter, a rewrite of 76 debug specs, and would have put a Wi-Fi SSID in a project file that can be opened against a board with no radio. What had to converge was the screen, not the storage. Writes to a vendor screen read-modify-write the section: `setVendorScreenData` replaces it wholesale, and the form layout only persists fields the user touched, so a blind write would drop the baud rate the Serial screen set. Enabling Modbus TCP also enables the Network section. The network is configured on a separate page the user may never open, and a board that compiled MBTCP with the network off came up and never linked. Routing: the Modbus vendor screen is re-homed under Servers, and the "+" card opens it instead of asking for a name and a protocol -- the board has exactly one Modbus server and it already exists. It keeps opening through its vendor-screen tab, so persistence, dirty tracking, save and revert are the paths that already worked; only the renderer changed. ProjectTreeBranch gains `forceExpandable` because the Servers branch counts PLCServers, and a baremetal board has none. Hardware settings are linked, not absorbed. Baud rate, RS-485 pin and Wi-Fi credentials belong to the package that knows the board; duplicating them here would give one value two owners. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The screen split gave the network its own section and its own switch, and nothing read that switch. A project with Modbus TCP enabled and Network disabled compiled MBTCP, called mbconfig_ethernet_iface at boot and never linked -- a healthy board that answers nothing, which on the bench reads as broken hardware. Only an explicit `false` blocks. The form layout persists only the fields the user touched, so a project where someone typed an SSID and never touched the toggle has no `enabled` key at all; refusing to build that would trade one silent failure for another. A pre-split project has no `network` section and keeps building byte-for-byte as it did. RTU is unaffected -- the gate is TCP's alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ompiles
DOPE-370. A P1AM-200 with 256 KB of SRAM served the same 56 coils as an Uno
with 2 KB, because MAX_DIGITAL_OUTPUT was a literal in openplc.h chosen per MCU
family. Every one of those macros is now `#ifndef`-guarded, and openplc.h picks
up a generated io_sizes.h when the project asked for more room.
io_sizes.h has to reach the build through openplc.h rather than defines.h.
openplc.h is included by ~20 HAL translation units that never see defines.h,
and defines.h deliberately carries no include guard so it can only arrive
through exactly one path (modbus_config.h explains why). A separately guarded
file is the only way to reach every consumer without breaking that rule; it is
pulled in behind `__has_include` so its absence is not an error.
The register store was capped at 255 in three separate places, all uint8_t:
init_mbregs's six parameters, MBinfo's six *_size fields, and the byte_addr /
pos indices into the packed bit banks. Asking for 256 coils wrapped to 0 and
every bounds check then rejected every address -- the shape of the P1AM report.
All are uint16_t now, which is also the natural ceiling: a Modbus address is 16
bits, so a count that did not fit could not be reached on the wire.
Two rules keep this safe:
* Growth only. These macros dimension the IEC pointer arrays as well as the
Modbus banks, and mapEmptyBuffers() aliases %MW/%MD/%ML into those banks,
so a segment shrunk below what a compiled program addresses would drop I/O
with no diagnostic. The floor is the board's firmware default.
* Silence by default. When every count equals the board default,
generateIoSizesHeader returns null, no file is written, and the firmware
compiles byte-for-byte as it did.
The ceiling comes from the board's package (`device.ioMax`). A board that
declares none cannot be raised at all, and the screen renders its counts
read-only rather than offering an input that cannot move.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…appear Reported on hardware: the Serial screen offered only a baud rate, no UART picker and no RS485 driver-enable pin, on both a NodeMCU and an ESP32 WROOM. Cause is the unified server screen itself. It renders in place of the whole Modbus vendor screen, so every field left in the `modbus_rtu` section stopped being drawn -- and `serial_port`, the secondary `baud_rate`, `enable_rs485_en_pin` and `rtu_rs485_en_pin` all lived there. The transports and the slave id are the native screen's to own; the wiring is not, and it was swallowed with them. Those four move to the `serial` section, which is where a user goes looking for them anyway: it is the page about the physical line. `modbus_rtu` keeps `enabled` and `rtu_slave_id`, both of which the native screen does render and both of which the device debug specs $ref. The compile step reads the new location first and falls back twice -- to `modbus_rtu.serial_port` / `.baud_rate` for a project saved against the first split, then to `rtu_interface` / `rtu_baud_rate` for the original single screen -- so no project loses its wiring. Also fixed, same report: the "Serial port & RS-485" button opened the Modbus screen, which is the screen it was on. It is gone; the two remaining buttons go somewhere. And navigation now calls setSelectedTab as well as setEditor -- swapping what renders while the tab strip still highlights the tab you left reads as a button that did nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tor's port Two things, both about the serial port. **Migration.** The RTU wiring moved from `modbus_rtu` to `serial`, and a project saved before that still carries it under the old keys -- where the Serial screen no longer looks but the compiler's fallback still does, so the screen and the firmware disagree with nothing to say so. `migrateModbusSerialFields` folds the four fields forward, handling both older spellings, and drops every stale key so the fallback cannot resurrect one on a later build. It runs when the board list lands, not on project load, because it has to know which installed packages ship a `serial` screen and the project reaches the store first -- the same reason `setAvailableOptions` already owns the deferred work in this slice. A board whose package is still unsplit renders the old screen and is left alone: migrating it would mirror the bug rather than fix it. Idempotent, and returns identity when nothing moved, so an already-migrated project is not touched on every board refresh. **The editor owns the default port.** The always-on debugger, the status and the licensing function codes all answer on the default UART, and it is where the USB cable lands. A second Modbus master cannot share that line, so the RTU slave needs a port of its own. The picker now shows that port greyed out and labelled, rather than omitting it -- a choice that vanishes leaves the user hunting for it. `FieldOption` gains `disabled`, the form layout renders it, and the list comes from a derived `board.modbusSerialPorts` so no package has to restate what the editor already knows from `serialPorts` + `defaultSerial`. It follows that a board whose only UART is that port cannot serve RTU at all while staying reachable from the editor. Those boards -- NodeMCU, Uno, Nano, every single-UART part -- no longer offer the transport, and the screen says why instead of leaving a gap. A board that declares no UART set keeps offering RTU: 13 of them do not, because their variants were never confirmed against hardware, and refusing there would remove a configuration that works today. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The whole configurable-I/O path was dead. `BoardInfoResolver` read `device.io` and `device.ioMax` off the manifest, and the adapter that turns a `BoardBuildInfo` into the pipeline's `BoardHalsBuildEntry` dropped both -- so `boardEntry.io` was always undefined, the pipeline skipped `io_sizes.h` entirely, and a project that had raised its counts compiled at the board defaults. Nothing said so. The compile succeeded, the sketch linked, and the reported RAM figure did not move. It is the same failure the `boardManagerUrl` comment three lines above describes, in the same object literal. Found by compiling for real rather than reading the diff: an ESP32 WROOM at the declared ceiling reported 47,520 bytes of global RAM -- byte for byte what it reported at the defaults. With the forward in place the same project reports 54,768. The 7,248-byte delta is exactly 1,812 new IEC pointers at four bytes each, which is the arithmetic the ceilings were derived from. Two regression tests: one asserting both fields arrive, one asserting they stay absent for a package that declares neither, since absence is the signal the pipeline reads to skip emission. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The note beside the Modbus RTU bring-up said running the debugger on the default serial while the RTU used a different UART "would need a second serial handler -- a documented follow-up". That handler landed in 4b3c138: `handle_serial` polls both ports under MBSERIAL_ON_SECONDARY, each with its own RX assembly buffer, and DEBUG_IFACE.begin() runs ten lines above. Worth correcting now rather than later, because DOPE-442 makes that case the normal one: the editor's connection occupies the default port, so the RTU is expected to take a UART of its own. Verified on an ESP32 WROOM, not by reading: RTU on Serial2 at 19200 with the debugger on Serial at 9600 answers FC 0x47 and 0x46 on slave 7, and the same board is silent when probed at 115200 -- so the rate really is the one the Serial screen set, not the core's boot-log default. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The packages rebuilt for DOPE-442 depend on editor behaviour that does not exist before this branch: `optionsRef` resolving `board.modbusSerialPorts`, a `disabled` select option, the unified Modbus server screen and the vendor-screen migration. Installed against an older editor the optionsRef falls back to the static list, which offers UARTs a board does not have and lets the user hand Modbus RTU the port the editor is talking on -- the exact bug this work removed, reintroduced silently. `minEditorVersion` in those packages is what refuses that install, and it needs a version to point at. Both files move together: APP_VERSION is what the About dialog shows, package.json.version is what electron-builder stamps and what the release tag must match, and shipping one without the other is how 4.2.7 and 4.2.8 went out with About stuck on 4.2.6. Minor rather than patch: the screen split is a breaking change for any package that has not been rebuilt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The pure helper had twelve cases; none of them exercised the part that can actually go wrong in the product, which is WHERE the migration runs. The project reaches the store before the board list resolves, so a migration hung off project load would see no Serial screen anywhere and skip every board, permanently. Two cases through setAvailableOptions: a board whose package ships a Serial screen gets its wiring folded forward and its stale keys dropped, and a board whose package has not been split is left exactly as it was. Negative control: neutralising the call in the slice fails the first and leaves the other 133 green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CLAUDE.md claimed 100% functions/lines/statements on four directories. jest.config.json enforces 75-98 depending on the directory, and gates branch coverage nowhere. A contributor reading the old text would either believe a new file must reach 100% or, worse, trust that the suite guarantees it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
APP_VERSION and the root package.json read 4.3.0 while release/app/package.json sat at 4.2.2 and both lockfiles trailed further behind. electron-builder reads release/app/package.json — directories.app points there — so a local package build stamped the wrong version on the binary. Set with `npm version 4.3.0 --no-git-tag-version --allow-same-version` at the root and in release/app, which updates each lockfile too. Version fields only; no dependency added, removed or upgraded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ation Brings the located-arrays work, the licensing and retain changes and the debug-transport rework onto the branch. Conflicts were in the files both sides touched: the target-capabilities resolver and its presets, the device slice, the native-screen enforcement hook, the compile pipeline and the firmware bundle step, plus the version fields and the explorer tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A baremetal target had no server element. Its Modbus lived in the modbus_rtu and modbus_tcp sections of the board's VPP screen, which the package ships whether or not anyone wanted a server -- so every such board appeared to serve Modbus, and nothing could be named, created or deleted. Protocol configuration belongs to the editor, so ModbusSlaveConfig now carries what a serial server needs: the transports it answers on, the slave id, and the RTU wiring, mirroring the fields the master already had. Transports is a set rather than the single value the master carries, because RTU and TCP together are one server listening on two wires, not two servers. A project saved by an older editor is promoted on open, at the first point where both the project's servers and the screen state are readable. The migration copies rather than moves: the sections stay until the compile pipeline reads the server instead, because stripping them now would produce a project that opens fine and compiles to a firmware with no Modbus at all. The Runtime v4 plugin's conf/modbus_slave.json is a contract with software already in the field, so the new fields must not reach it. The generator builds an explicit shape rather than spreading the config, and a test pins that. DOPE-442 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Modbus server profile carried a `store` discriminant so the screen could read a baremetal board from its vendor screen and a Runtime v4 target from the project. There is one store now, so the discriminant went: what the profile answers is which fields a target lets the user set, which is a different question and the only one left. The editor's own UART is offered as an RTU interface on every board. The firmware serves the debugger and the register table on it together, so using it is a legitimate choice -- which of the two the user talks to at a given moment is theirs to arrange, and refusing it was the editor deciding for them. The refusal and the explanation it printed are both gone. That makes two fields the user must not touch there. The slave id and the baud rate of the default port are what the editor dials to reach the board, so they are shown read-only carrying the board's own value; changing them would leave the board unreachable with nothing to say so. The transports become a choice between the sets the board offers rather than two independent switches. Two switches can both end up off, which would be deleting the server from inside its own editor -- deleting is the explorer's Delete, and the tree now lists an ordinary project element rather than a re-homed vendor screen. Raising the buffer counts is no longer offered: the same MAX_* dimension the IEC pointer arrays and mapEmptyBuffers() aliases the memory segments into the Modbus banks, so it is an I/O-image change rather than a Modbus setting. The screen shows the sizes and the map they produce. DOPE-615 owns raising them. DOPE-442 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ackage The defines emitter read what was served out of the board's VPP screen, which is where a baremetal board's Modbus used to live. It reads the project's server now. The screens keep what they own -- the UART's speed, the RS-485 pin, the network -- so the two halves of the block come from the two sides of the boundary rather than both from the package. The editor's own link is pulled out of that. DEBUG_BAUD and DEBUG_SLAVE come from the serial screen, never from a server. Welding the debugger's slave id to the server's made every change to that field an access event, and would have left a project with no server unable to debug at all now that a server is something the user creates rather than something every board has. When the RTU shares the default UART the two are one listener with one id, so the editor's wins for both -- otherwise the board would answer the bus on one id and the editor on another, with only one of them able to be right. A firmware build serves exactly one slave: modbus.slaveid is a single global and init_mbregs is called once. A build with more than one enabled server is refused, naming them, because "only one is allowed" leaves the user to guess which to turn off. Creating several stays allowed -- a project moves between targets, and a server a Runtime v4 build serves happily should not have to be deleted to build for a microcontroller. The TCP listen port travels with the server as MBTCP_PORT. The legacy reading arm for the RTU wiring is gone, since a migration moves those fields; the legacy network fields stay readable, because nothing migrates those and dropping them would lose a pre-split project's Wi-Fi credentials on upgrade. DOPE-442 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The single-serial path assembled straight into mb_frame, which is only safe while nothing else writes it between scan cycles. TCP does: mbtask() runs handle_tcp() first, so an incoming request overwrote a partial serial frame while mb_rx_len still described it. The framing logic then resynced a byte at a time and the in-flight transaction was lost -- intermittent, and worst under exactly the concurrent TCP load a working installation produces. That path gets a buffer of its own wherever TCP is in the build, which is what the dual-serial path has had all along. The condition is MBTCP rather than MBSERIAL && MBTCP because the always-on debugger assembles through the same path and was losing frames the same way in a TCP-only Modbus build. Measured on an ATmega328P: 128 bytes, 186 to 314 of 2048. It is compiled only where TCP is present, so a board without a network keeps its footprint. The TCP listen port stops being hard-coded in three places and honours MBTCP_PORT, falling back to the IANA default so a firmware built by a toolchain that does not emit it listens where it always did. DOPE-442 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The profile resolver decided a board was baremetal when its package shipped a Modbus screen. That worked while the screen held the board's Modbus configuration, and stops working the moment it does not: a package with nothing left to put there ships no such screen, and every arduino board would have resolved as a Runtime v4 target -- offering a configurable port and user-sized buffers that the firmware does not have. The compiler is the fact underneath, and it does not move. A package that still ships a Modbus screen is accepted too, so one published before the split keeps working. DOPE-442 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The packages published for this work declare a 4.4.0 editor floor, so an editor still calling itself 4.3.0 refuses them. APP_VERSION is what the About dialog shows and package.json is what a local build stamps; release/app is what electron-builder reads. All three say the same thing. DOPE-442 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…not delete Three things the screen got wrong when it met real use. The transport was a row of buttons. It is a dropdown: three options in a fixed order on every target, with the ones a board cannot serve rendered disabled rather than dropped. A control that changes shape between targets cannot be compared across them. Turning Modbus off meant deleting the server from the project tree, which threw away the slave id, the port and the buffer sizes with it. There is a master switch now. Off keeps every setting and serves nothing, which is what someone commissioning a panel wants far more often than starting over. Everything else that used to disappear now arrives disabled with the reason: the serial port and slave id on a target with no RTU, the network interface and port where the firmware fixes them, and the Hardware Settings card, which used to vanish entirely on Runtime v4 and took the screen's shape with it. The buffer panel shows every segment, including the ones a target has no storage for -- sized zero, read-only, saying so. Hiding %MX on baremetal left no way to tell "this board has no %MX" from "the editor forgot about %MX", and it was the last thing making two targets' screens differ in shape. DOPE-442 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A board serves one Modbus transport at a time. Offering both invited a configuration that the firmware's design never assumed and that nobody asked for, so the dropdown no longer lists it. It is not removed outright. A project can already be in that state -- a pre-4.4.0 board with both screen toggles on migrates that way -- and a dropdown that answered "Modbus TCP" for a server actually serving both would be lying about the configuration it is there to show. So the entry appears when it is the current state, disabled, with a hint saying to pick one and that nothing else is lost by doing so. DOPE-442 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…glyph Every row on the server screen printed its explanation beside the control. A settings screen that explains itself in full sentences on every line stops being a settings screen: the controls no longer line up and the eye has no column to run down. The help moves behind the same info glyph the VPP screens already use, and that glyph moves into the tooltip atom so there is one of it. The native screens and the package-declared ones should be indistinguishable to use, and two copies of the same control is how that stops being true. DOPE-442 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Behind a hover glyph the text is read on purpose rather than skimmed past, and a sentence that earns its place there is one the reader can take in without stopping. Every hint on the screen is now that length. The slave id no longer warns about leaving the board unreachable. The next round gives the debugger a dedicated id that the server always answers alongside whatever is configured here, so the hazard the sentence described stops existing -- and the read-only rule on the editor's own port becomes a workaround with a known end date rather than a property of the design. DOPE-442 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Serving both is offered again. The constraint that motivated hiding it was the firmware's, and it is gone: the single-serial path has its own RX assembly buffer now, so a TCP request no longer overwrites a serial frame held across scan cycles, and the two transports coexist on a board that has both. With the option back, nothing special is owed to a project already serving both: it is a supported configuration rather than a state to be migrated out of, so the dropdown carries no legacy entry and the hint no longer asks the user to pick one. DOPE-442 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DOPE-619 `handle_serial_port` accepted a frame only when its first byte matched one compiled-in slave id. On the default UART that single id served both the editor's link and the user's Modbus server, so the two could not have different addresses and the editor's won — which is why the slave id is read-only on that port. It now takes a second id. A frame matching the server's is processed as before; one matching only the editor's must carry an editor function code and is otherwise dropped in silence, because the channel is private and an exception would tell whoever is scanning the bus that the address is live. Pass the two equal, as every port the editor does not sit on does, and nothing changes. The function-code test is its own predicate rather than `mb_pdu_skips_crc()`, which answers a different question and excludes 0x4B: reusing it would have left run/stop unreachable on the editor's id. `exceptionResponse` wrote the server's id over the received one. Invisible while the two were the same value; with two ids it answers as the wrong slave, and over TCP it overwrote the MBAP unit id the client sent.
DOPE-619 The editor's slave id was a package field, `screens.serial.slave_id`, and on the default UART it overrode whatever the Modbus server was set to — one listener, one address. That is what made the field read-only there. The firmware answers both ids on that UART now, routed by function code, so: - `DEBUG_SLAVE` is the constant 1. It cannot be configured because there is nothing left for a configuration to decide: whatever value it held would have to match a firmware the user cannot see, and a mismatch reads as a healthy board answering nothing. - `MBSERIAL_SLAVE` is the server's id on every port, including the default one. - the field on the Modbus server screen is editable on every port. Setting it to 1 is not a collision: the standard codes and the private ones do not overlap. A board flashed before this answers only the id its project recorded, and it fails as silence, which reads as a dead board rather than a stale address. So Connect tries 1 and then that id, as one speculative candidate at the declared baud rate. Pairing it with the swept rates instead would cost five more port opens, and an open resets an AVR or ESP8266 — restarting the user's program to chase two stale values at once. The legacy id is read from the project's own leftover screen state rather than declared in the manifest: the packages' ref check rejects a `$ref` pointing at a screen they no longer ship, and the Modbus screen is exactly such a screen.
DOPE-442 Which UART Modbus RTU answers on was already the server's choice, but that UART's speed stayed in the package's serial screen. One line, two owners, split across a project setting and a board setting — and a board switch had no way to say which won. The server carries it now, on any UART that is not the default one. The default one keeps the package's value and the field is shown disabled with the reason: it is the editor's own line, one UART has one speed, and unlike the slave id no amount of firmware routing makes two possible. The resolution moves to `middleware/shared/utils/modbus-server-profile/baud.ts` so the screen and the emitter call the SAME function. A hook may not import from `backend/shared`, so leaving it there meant two copies of one fallback chain — and a screen quietly disagreeing with the firmware is the failure this area keeps producing. `serial.modbus_baud_rate` survives as the fallback rather than as a field, so a project that already carries a server with no speed of its own keeps building what it built yesterday.
…lt one's DOPE-442 The compile-pipeline section listed the UART's speed as wholly the package's. Half of it is the server's now, and the split has a reason worth stating: the default UART carries the editor's own link, and one UART has one speed.
DOPE-442 The way out to the board's serial and network pages sat in a card of its own, two panels below the fields it explained. Someone looking at a baud rate they cannot edit had to read the tooltip, scroll past Buffer Mapping, and find a card whose title named none of it. The panel now carries three groups - the server, the serial line, the network - and each hardware group holds its link in its header, where a section action belongs. The card is gone; its two buttons are those links and its sentence is the reason each one carries when disabled. Grouping pays for itself twice. The flat list never said which rows were RTU and which were TCP, and that started mattering the day a server could serve both at once. The groups are named for the wire rather than the protocol, deliberately: their rows enable on what the board HAS, not on what the user selected, so "Modbus RTU" would promise a coupling the screen does not implement. On the default UART the baud rate stops being a dead dropdown and becomes the route to the page that sets it, showing the value and who owns it. Only where such a page exists - otherwise there is nowhere to go and it stays a plain disabled field.
…ttings DOPE-442 Three ways a package installed before 4.4.0 goes wrong under an editor that has already upgraded. The version floor only guards the other direction, and the package catalogue that would offer an update does not exist yet, so the editor has to tolerate the old package rather than expect it to be replaced. **Connect skipped the id every firmware it builds answers on.** The fallback compared the declared slave id against the project's legacy one and offered the legacy id only when they differed. On a pre-4.4.0 package both come from the same screen key, so they never differ and the editor dialled the old id alone — while a board reflashed since answers only 1. It now plans both extra ids: the editor's constant and the project's recorded one, deduplicated, at the declared baud rate. A board on the declared id still pays nothing. **A missing board page had one explanation for two situations.** "The host operating system owns this target's ports" is true of Runtime v4 and false of an ESP32 whose package predates the split. The pre-4.4.0 package still ships the Modbus screen it used to own, which is what tells the two apart. **The Modbus vendor screen was filtered out of the tree.** It was filtered because it used to be re-homed under Servers, and that re-homing is gone. On a current package there is nothing to filter. On an older one it hid the only view of state that still reaches the firmware: with no server in the project, the defines emitter falls back to those very sections.
DOPE-442 Listing that screen in the tree again was half a fix. Opening it still routed to the native server editor, which reads its server name from a `plc-server` tab and gets none from a vendor-screen one — so it fell through to "The selected target does not serve Modbus. Pick a board that does, or install the vendor package", both halves of which are false in front of a board that does and a package that is. The interception was the other side of the filter: while the leaf was hidden, the only way in was a re-home, and the branch was all but dead. The leaf is back, so the screen renders declaratively like every other one the package ships. On a package built for 4.4.0 nothing changes: there is no Modbus screen to route.
…e its sizes DOPE-442 Found in the field on a real upgrade: the editor moves to 4.4.0, the package stays on the version production serves, and the Modbus server screen shows no counts and no address map at all - on a board that plainly has both. The `io` block those counts come from is new in 4.4.0. A package published before it declares nothing, and the screen answered that by withholding the map and telling the user to update the package. But a board whose package says nothing is not a board of unknown size. It is a board compiling with the `#ifndef` fallbacks in `openplc.h`, which has exactly two branches - and every one of the 76 devices in the catalogue declares an `io` block matching one of them exactly. The packages are transcribing the firmware, not adding to it, so the map is derivable and it is the map the board serves. Which branch applies is chosen by a macro the compiler defines and the editor never sees, so the four devices that land in the small-AVR one - Uno, Nano, Leonardo, Micro, the ones with no %MW, %MD or %ML at all - are named by device id. That list is not a guess: it is the four whose declared sizes match that branch, and it is consulted only for a package too old to declare them, which comes from the same catalogue. Two things the fallback deliberately does not do. It states no ceiling, because `ioMax` raises a floor the package declared and a limit nobody stated is not a limit. And it does not pass itself off as declared: the screen says the sizes are the family's defaults and that updating the package reads the board's own.
DOPE-442 The CI format check runs `prettier --check` over all of `src/`, and it is a separate gate from lint — `eslint --fix` does not apply it. Four files went out unformatted: the two utils added for the Modbus server profile, and the two components edited around them. Whitespace only; no behaviour and no semantics change.
DOPE-442. The migration that promotes a baremetal board's Modbus screen to a
PLCServer read the UART and its speed from `serial.modbus_port` and
`serial.modbus_baud_rate` only. Neither key exists yet at the moment it runs.
Two migrations touch those values and they run in the wrong order for each
other. `planVendorModbusMigration` runs on project load; the one that folds
`rtu_interface` / `rtu_baud_rate` forward into `serial` runs later, when the
board list resolves, because it has to know which installed packages ship a
`serial` screen. On a board whose package was never split it is skipped
entirely and the keys stay under their original names for good.
So the migrated server came out with no serialPort and no baudRate on exactly
the projects the migration exists for, and the board then answered at the
package default instead of the port and speed the user had configured. The
plan now reads all three spellings in the same precedence the fold uses, and
an empty value falls through instead of winning.
Alongside it, six things a review of the same area turned up:
* the buffer-mapping toggle carried `aria-label` on the `<label>` wrapping a
`sr-only` checkbox, which names nothing -- the control had no accessible
name at all;
* a server's `baudRate` was `z.number()` while every field around it is
range-checked, so a fraction or a negative reached firmware generation;
* `resolveDefaultPortBaud` used `??`, which returns a persisted empty string;
this is the number the editor dials to reach the debugger, so a bad one
locks the board out silently;
* `BoardInfo` could not name `ioMax`, though the resolver and the hardware
module both forward it and the Modbus profile reads it to derive the
ceilings -- it was crossing the contract on structural typing alone;
* a manifest declaring only part of an `io` block was cast to a complete one,
and clamping against an undefined default yields NaN, which compares
unequal to everything and emits an override nobody asked for;
* CLAUDE.md described the coverage gates as directory floors and then, nine
lines later, demanded 100% per file.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…42-modbus-server-unification
…ion creates DOPE-442. Two ways a project's Modbus ended up disagreeing with the firmware. `selectModbusServer` reported nothing for a server whose `enabled` was false, and `generateModbusDefines` reads an absent server as "this project predates the server element" and falls back to the board's screen sections. Those sections still carry `modbus_rtu.enabled: true`, and on a board whose package was never split nothing ever clears them, so switching the server off compiled RTU anyway. Existing and serving are now separate questions: a server that exists and serves nothing comes back with an empty transport list, which the emitter already handled and documented but could never receive. Only a server on the new model can do this -- a pre-4.4.0 `modbus-tcp` server carries no `transports` at all and must keep falling through to the sections, or a project that has always compiled from them goes silent. The server the migration promotes was created after the file registry is built from the loaded project data, and it is not in that data. It therefore had no registry entry at all: invisible to dirty tracking, to the close-project check and to the single-file save. The project looked clean, closed without a prompt, and nothing was written -- so the migration ran again on every open while the board kept compiling from sections the editor no longer shows. It is now registered unsaved, and the workspace says so. Verified by neutralising each fix in turn: the disabled-server assertions fail without the selector change, and the registry assertion fails without the editing-state change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DOPE-442. `io` / `ioMax` in the VPP manifest and the whole `io_sizes.h` chain
come out. DOPE-615 owns sizing the I/O image from the project, and lists
"reintroducing a per device image size in the VPP manifest" as out of scope --
so shipping both would put two demands on one surface, with this one declaring
per-device numbers that the other is designed to derive.
What goes:
* the manifest's `device.io` (76 devices) and `device.ioMax` (57), their
schema definitions, and every hop that carried them: board-info-resolver,
hardware-module, resolve-board-selection, the pipeline inputs, BoardInfo;
* `generate-io-sizes.ts` and the emission of `src/io_sizes.h`;
* the profile's `derivedCounts` / `minCounts` / `maxCounts` / `countsSource`
on baremetal, and the amber caveat the screen printed when a package
declared no sizes.
What stays, deliberately: the Runtime v4 half of the buffer-mapping block. Its
counts come from the server's own `bufferMapping` and reach the device through
`conf/modbus_slave.json`, so they are neither derived from a manifest nor
anything DOPE-615 touches.
The firmware files go back to `development` untouched. The `#ifndef MAX_*`
guards, the `__has_include("io_sizes.h")` hook and the uint8_t -> uint16_t
widening of the register store all came from one commit that belongs to
DOPE-370, and openplc-editor#1079 is open with the same three files -- keeping a
second copy here would only conflict at merge.
One thing this restores rather than removes: with no sizes to report, the
screen's "This target reports no Modbus buffer sizes, so no address map can be
shown" branch is reachable again. That is what the demand's AC04 specified in
the first place.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DOPE-442. The compiler reads the project from disk; the migration that folds
`rtu_interface` / `rtu_baud_rate` / `rtu_rs485_en_pin` into `serial` runs in the
store. So the emitter meets the pre-split spellings routinely -- before the
first save, always in a CLI process, and permanently on a board whose package
was never split -- and it had stopped reading them.
The RS-485 driver-enable pin was the worst of it: no `MBSERIAL_TXPIN`, the
transceiver never asserts DE, and the board receives every request and answers
none. Silent, with no warning on any path and no second route, since the server
config has no RS-485 field.
The default UART's speed was wrong in both directions. The fold deletes
`baud_rate` / `rtu_baud_rate` and writes `modbus_baud_rate`, which nothing read
back -- a project running at 19200 silently became 115200 and an already-flashed
board went unreachable. And the guard that kept an RTU on a SECOND port from
setting the default port's speed was gone, so a project with RTU on Serial1 at
9600 brought the USB port up at 9600.
One resolver now answers all three questions -- which UART, is it the default
one, what speed -- and the screen and the emitter both call it. Deriving them
separately is how a screen ends up showing a read-only baud for one port while
the build emits `MBSERIAL_ON_SECONDARY` for another.
Alongside, five things that shared the same review pass:
* a newly created Modbus server seeded no `transports`, so the build could not
see it, fell back to screen sections a 4.4.0 package does not ship, and
emitted no Modbus at all -- while the screen, defaulting the same field,
said "Serving Modbus TCP";
* Modbus TCP was offered enabled on every arduino-cli board, including the 29
that declare no network hardware, where it emits a stack the firmware has
none for. It is gated on `networkInterfaces` now, and the new-server form
stops offering S7comm and OPC-UA on a target that declares neither;
* the baremetal TCP port was rendered "Fixed by the firmware", which stopped
being true when the firmware started reading `MBTCP_PORT`. A project carrying
8502 showed 502 and flashed a board listening on 8502;
* the migration could mint a server its own schema rejects, and the parser then
skips it with a warning -- so it reappeared as unmigrated on every open, the
project was dirty every time, and the compile fell back to the sections;
* `port` and `slaveId` were unbounded where every sibling is range-checked;
`slaveId` admitted 0, the broadcast address a server must never answer on.
Firmware: the function-code split routed one way only. On the server's public id
the editor's codes were dispatched like any other, which is the opposite of what
this firmware documents. They are refused there now, and CRC is skipped only on
the editor's own id, so the refusal is never answered to a frame nobody checked.
Not verified on hardware.
Coverage: `src/middleware/shared/**` and `src/frontend/hooks/**` were collected
by nothing and floored by nothing, which is where this demand's largest new
module lives. Both are measured now, the first with a real floor including
branches, the second as a ratchet at what it measures today.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…order DOPE-442. Four criteria were asserted in prose and nowhere in code. `NFR03` -- promoting a project's Modbus to a `PLCServer` does not change the firmware -- is now an A/B: one project's configuration emitted through the legacy screen-section path and through the migrated-server path, compared directly. It is the only form of that assertion which stays true as either path changes. The two-enabled-servers REFUSAL had no test; only the selector that detects the clash did, and the refusal is what the user meets. Both directions are covered: a baremetal build stops and names both servers, and a target that never reads the selection builds anyway. `deleteServer` never asserted `pendingDeletions`, though every sibling does. Without that entry the save leaves the file on disk and the server returns on the next open. And a resolver test asserted the same expression twice. The CLI loaded boards BEFORE the project. One migration hangs off that action and reads project state -- the fold of the pre-split `modbus_rtu` wiring keys into `serial`, gated on the board's package shipping a `serial` screen -- so in a CLI process it ran against an empty store and never again. The GUI sees the project first and the boards after, so it fires once there; the two could therefore emit different `defines.h` for the identical on-disk project. The board list is re-applied after the project lands, which is idempotent by construction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…42-modbus-server-unification # Conflicts: # src/frontend/store/slices/shared/slice.ts
DOPE-442. Taking the I/O sizing out and resolving the `development` merge left three orphans: `cn` and `useMemo` in the vendor-screen form layout, whose only user was the greyed-out-UART block, `ModbusSegmentCounts` in the profile resolver, and an import order the merge broke in `generate-defines.ts`. Worth recording why only one repo caught it: the editor's tsconfig sets `noUnusedLocals: false` and web's does not, so an unused import here is a lint error there AND a type-check failure -- the mirror turned a tolerated warning into a broken build on the other side. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DOPE-442. `Object.hasOwn` needs `lib` es2022 and the web build's is below it, so `pnpm tsc --build` failed there while `tsc --noEmit` passed here. Same guard, written as `Object.prototype.hasOwnProperty.call`. The guard is not incidental: `lookupPath` walks a dotted `optionsRef` across an object the package supplies, so without it an `optionsRef` of `board.constructor` would resolve up the prototype chain. Worth recording that the editor's own type check does not run the same command the mirror's CI does -- `--noEmit` here, `--build` there -- which is how this reached CI green on one side and red on the other. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…dbus-server-unification' into RTOP-285-merge-dope442
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
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
2 participants
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.
Merges
feature/DOPE-442-modbus-server-unification(PR #1094, still in review) into our S7Comm feature branch, so baremetal targets stop being refused server configuration.What this fixes, and what it does not
The editor gates the Servers UX on
!modbusTcpServer && !opcuaServer && !s7Server. DOPE-442 flipsARDUINO_CLI_CAPABILITIES.modbusTcpServertotrueand addsmodbusRtuServer, because the baremetal firmware always served both — the flags readfalseonly because the config lived in a vendor screen the Servers UX could not see.Resolved for a Siemens LOGO! 8.2 after the merge:
modbusTcpServermodbusRtuServeropcuaServer/s7ServermodbusTcpRemote/ethercat→
targetCantHostServersfalse (servers allowed) ·targetCantHostRemoteIotrue (remote devices still blocked)Remote devices are not fixed by this and cannot be. Modbus master on baremetal is explicitly out of DOPE-442's scope; it is DOPE-372, which is In Development with no branch pushed yet. Until that lands there is nothing to merge for it.
The merge itself
Clean — no conflicts, in either repository. Both sides survived intact:
modbusTcpServer: true/modbusRtuServer: trueon the arduino-cli presetopcua/s7profiles resolved on every path,DEFAULT_S7_PROFILE.szlon, the most-compatible defaultsopenplc-packagesis deliberately not merged: its DOPE-442 PR (#48) touches 61 files across the other packages and does not touchcom.siemens.logoat all, so the LOGO! VPP needs no rebuild or re-sign. The LOGO! still declares an old-style (unsplit) Modbus screen, which DOPE-442 handles explicitly — "a package that has not been migrated is unaffected."Verification
Gates:
tscclean (bar two pre-existinguploadMethoderrors) ·validate:archclean · jest 8,391 passed, 0 failures across 402 suites.Hardware, Siemens LOGO! 8.2 — compiled, flashed and exercised, with no "Modbus Server is only supported on OpenPLC Runtime v4" warning in the build log any more:
Image grew 112 bytes (227,956 → 228,068) from DOPE-442's firmware changes — the dedicated single-serial RX buffer and configurable
MBTCP_PORT.One pre-existing problem this surfaced
compare-surfaces.pyreports 9 diffs between editor and web on this branch, and none of them are DOPE-442's or ours. They are LOGO! work that was never mirrored to openplc-web — the Ethernet upload/connect UI (isEthernetUploadTarget, from2ccd440c6) and the bootloader function codesMB_FC_REBOOT_BOOTLOADER/MB_FC_GET_LOCK_STATEin the bare-metal Modbus headers. Present onRTOP-285-opcua-baremetal, absent fromdevelopmentand from web.DOPE-442 itself reports 0 diffs. This will have to be resolved before this lineage reaches
development, and it is worth deciding deliberately whether web should carry the Ethernet-upload UI at all rather than mirroring it blind.Risk worth naming
DOPE-442 is still in review. Merging it here couples this branch to a branch that may still change; if it lands on
developmentin a different shape, this merge will need revisiting.🤖 Generated with Claude Code
https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p