Repository navigation
Let the template update a block a printer already has - #123
Merged
Merged
Conversation
The toolchange retract fix had to ship as a renamed block -- #@retire auto_mode plus #@feat auto_mode2 -- because the merge moves whole blocks and has no way to change one that is already on a printer. That works, and it is wrong: it renames a feature the owner can see, for a reason that is none of their business, and it takes the block carrying both print macros off the printer for the length of a merge to put a corrected copy back. So the template can now carry #@reViSe <feature-id> | why and the printer's copy of that block is replaced, in place and at the same position in the file, by the template's current version. auto_mode keeps its name and gets the fix. This is the only operation that reaches inside a block, so it is narrow by construction: * It acts only on a block that is ALREADY THERE. It never adds and never resurrects -- a block the owner deleted by hand stays deleted, which is still the seeded record's decision, not this marker's. * The owner's toggle is carried over. A body that was switched off comes back switched off, line for line the way addon_features.set_block writes it. Verified through addon_features itself: --list reports OFF before and after, and switching the feature back on afterwards yields the NEW body with no sentinel left behind. * A block that already matches is not touched and not reported, so the marker can stay in the template instead of being cleaned up a release later. * The #@ENDFEAT guard applies as everywhere else. With the terminator deleted by hand the run refuses and the file keeps all 68 of its sections. * Only the sections a newer version GAINS are checked for collision; the ones the block already owns would otherwise read as a clash with itself. A revision that would add a section living elsewhere is skipped, not applied. THE WRITER IS NOW ONE WALK. Removals used to be a set of line numbers filtered out and additions a tail append. A replacement is neither, and doing the three in sequence would shift every span measured after the first edit. So drop, loose and revise go into one edit map keyed by original line number and the file is walked once. An overlap check sits on it that should never fire -- blocks do not nest and #@drop only names sections outside them -- and refuses the whole run rather than writing a half-applied file. THE GUARD HAD TO LEARN IT TOO. apply-addon-merge.sh starts python only when a feature id is MISSING, and a revision adds no id -- so without this the mechanism would never have run in the field at all. It now compares the block itself, sentinel stripped, and starts the merge when it differs. Measured: an out-of-date block starts it, and once revised the guard produces no output at all, so python is not started on every klipper start. The control case -- a block edited to look stale again -- starts it once more, so that silence means current rather than blind. WHAT IT COSTS, documented in the header rather than discovered later: a revised block belongs to the kit, not to the owner. Hand edits inside it are overwritten the next time the template moves -- loudly, and with the previous whole file kept as .pre-merge.bak, but overwritten. Checked: the owner's line is gone from AddOn.cfg and present in the backup. Every case above was run against the real AddOn.cfg of a running printer, not a fixture. Fresh installs stay silent: install-addon-cfg.sh copies the template verbatim, so the marker lands in the owner's file, where it is inert -- markers are read from the template only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
addon_merge guards itself well at run time: an unpaired #@drop is refused, a #@retire for a feature the template still ships is ignored, a #@reViSe naming a block that is not there is skipped. Each is the right thing to do on a printer, and each is silent to whoever wrote the marker -- the message is printed in a journal in somebody's workshop, not in the pull request that got it wrong. The P114 gate ran two versions out of date for months for that kind of reason: the delivery looked like it was working. The same rules, checked where a typo is still cheap. Seven of them: a block that is never closed or that opens while another is still open, a feature id declared twice, a #@retire for a feature still shipped, a #@reViSe with no matching #@feat, #@reViSe and #@retire naming the same id, an unpaired #@drop, and a section declared twice in one template. Each rule has a control case that must fail, run against a copy of the real template with exactly that damage introduced: all seven are caught, and the undamaged template still passes. A gate that can only say yes is worth nothing, which is why the controls exist rather than the passing run alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A design panel was run over the #@reViSe proposal on six lenses. Its own summary is worthless -- 16 of 38 agents died on a session limit, including every refuter for three lenses and the synthesis, so "no surviving concerns" means NOT CHECKED, not nothing found. The raised concerns were read and judged by hand instead. Most described the proposal as worded rather than what was built: the collision check does not copy the retire pop-loop, the writer does not splice in sequence, the off-state test is addon_features' own first-non-blank-line rule. Four were real. 1. A revision's newly gained sections were checked against `declared` but never written back into it, so an add later in the SAME run tested against a map that predated the revision. Control: without the fix a block adding the same section is merged and the file ends with two of it -- a config klippy refuses. With it the add is skipped and reported. 2. The retirement pairing rule looked only at `add`. A retirement whose sections a REVISED block takes over is a replacement just as much as one handed back by an addition, so it was held back and both copies stayed. Same control, same two-sections-in-the-file result without the fix. 3. auto_mode2 exists on alpha and nowhere else, but a printer that ever ran that channel has it. Restoring #@feat auto_mode without withdrawing auto_mode2 would leave such a printer carrying both -- two PHROZEN_TOOLCHANGE sections. The template now retires it, so those printers take the add path for auto_mode and lose auto_mode2 in the same run. Rehearsed end to end: alpha template first, then this one, ending with one of each section and the fix in place. 4. The guard and the merge disagreed on what "unchanged" means: body_key rstrips every line, the guard's awk did not. An AddOn.cfg edited on Windows carries a CR on every line, which would have made the guard call the block stale on every single boot and start python to be told there was nothing to do. The awk now strips CR and trailing whitespace the same way. Verified with a block given trailing spaces and CRs: the guard stays quiet. Also corrected a docstring that called its own trailing-blank trim redundant. It is not: blocks() returns an unterminated final block all the way to EOF. CI now refuses to ship a template like that, but the function should not assume it. While testing #122 earlier, a merge run against a scratch copy used the real HOME and appended auto_mode2 to the dev printer's addon-seeded-features -- a feature that printer never had. Removed; every run since uses an isolated HOME. 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
1 participant
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.
The toolchange fix shipped as a renamed block —
#@RETIRE auto_mode+#@FEAT auto_mode2— because the merge moves whole blocks and had no way to change one already on a printer. That works, and it is wrong: it renames a feature the owner can see for a reason that is none of their business, and it takes the block carrying both print macros off the printer for the length of a merge to put a corrected copy back.So the template can now carry:
and the printer's copy of that block is replaced in place, at the same position in the file, by the template's current version.
auto_modekeeps its name and gets the fix.Narrow by construction
addon_features.set_blockwrites it#@ENDFEATguardThe writer is now one walk
Removals were a set of line numbers filtered out; additions a tail append. A replacement is neither, and doing the three in sequence would shift every span measured after the first edit. So drop, loose and revise go into one edit map keyed by original line number and the file is walked once. An overlap check sits on it that should never fire — blocks do not nest,
#@DROPonly names sections outside them — and refuses the whole run rather than writing a half-applied file.The guard had to learn it too
apply-addon-merge.shstarts python only when a feature id is missing, and a revision adds no id — without this the mechanism would never have run in the field at all. It now compares the block itself, sentinel stripped.Verified against the real
AddOn.cfgof a running printer--listsays OFF before and after; re-enabling yields the new body, no sentinel left#@ENDFEATdeleted.pre-merge.bakWhat it costs — documented, not discovered
A revised block belongs to the kit, not the owner. Hand edits inside it are overwritten the next time the template moves — loudly, and with the previous whole file kept, but overwritten. The escape is to switch the feature off and keep your own version outside the block.
Plus: a CI gate for the markers
No gate knew the markers existed. Each of the merge's run-time refusals is silent to whoever wrote the marker — the message goes to a journal in somebody's workshop. Seven rules now checked in CI, each with a control case that must fail, run against a copy of the real template with exactly that damage introduced. All seven caught; the undamaged template passes.