Continuous bed mesh scanning with binary inductive probe#7224
Continuous bed mesh scanning with binary inductive probe#7224mairas wants to merge 5 commits intoKlipper3d:masterfrom
Conversation
src/probe_scan.c
Outdated
| enum { | ||
| RT_NO_RETRANSMIT = 0x80, | ||
| RT_PENDING = 0xff, // New data waiting to be sent | ||
| RT_ACKED = 0xfe // All data acknowledged |
There was a problem hiding this comment.
By design, data sent from the MCU to the host is not acknowledged or tracked in any way.
This is intentional because the MCU has limited resources; it should be simple, and the host should be able to figure it out.
It is much simpler and cheaper to do complicated stuff in Python than in the firmware.
https://www.klipper3d.org/Protocol.html#message-flow
If one wants to send a large quantity of data, it is better to reuse the bulk subsystem for that.
There was a problem hiding this comment.
Fixed. Sorry, should've been obvious.
src/probe_scan.c
Outdated
| // the timer callback will re-wake this task | ||
| ps->retransmit_state = ps->retransmit_count; | ||
| // Copy events under IRQ protection | ||
| uint8_t send_buf[MAX_EVENTS * EVENT_SIZE]; |
There was a problem hiding this comment.
You simply can't transmit 80 bytes in the payload in any direction.
IIRC, I would estimate the max payload size to be 64 - 3 - 2.
Where you have to account for all fields, so:
-2 for cmd, -2 for oid, -1 for ack_count, -1 for "events".
So, about 53 bytes for your payload.
|
Without getting into the weeds about the specific implementation, I would be extremely weary of merging AI generated code. I'm unsure if its compatible with the GPL because the AI cannot own the copyright. I don't believe anything has been decided in court but most likely all AI generated code will be considered public domain. |
0225867 to
dd5fbaa
Compare
If a coding agent would have created the PR without any human guidance or interaction, I would agree with it not exceeding the threshold of copyright. However, in this case, the algorithm design is solely mine, I have reviewed the code and modified many parts of the code. At that point, the coding agent is just a tool. It's still operated by a human, and there's no question of the output being my intellectual property and exceeding the threshold of copyright. Finally, even though IMHO not applicable to this case, public domain content is not incompatible with GPL. If you made a UI frontend that included the lorem ipsum template text, it would not invalidate the license of the surrounding code. Some auxiliary content like the PR description are almost completely Claude-generated, true, but they're not part of the codebase, either, and I'm not sure if they even have any license assignment in GitHub. Given the prevalence of agentic coding tools in the software industry, I would be fairly surprised if the intellectual property assets of the majority of software or consulting companies would suddenly be deemed public domain. |
c74d2b5 to
e1873a2
Compare
I have no doubt that you put great effort into the design and clearly the prompts would fall under your ownership and copyright. With regard to the result, I'm not entirely sure it is legally accurate to claim copyright. However if you feel that is the case then you should add a header to each of the new source files indicating your ownership of the copyright and sign off on each commit indicating acceptance of the DCO.
I agree that public domain content can be used in GPL licensed projects. The overall point I was attempting to make was that we do not yet know exactly how AI generated code will ultimately be classified until this issue is inevitably litigated. I'm speculating that AI generated code released to the public will end up being public domain, but I do not know that. FWIW, this is simply my opinion based on where things stand today, but the decision would be Kevin's. |
e1873a2 to
e49cacb
Compare
|
I've added the missing Signed-off-by lines. If you think any part of the PR commits is not viable as a Klipper contribution because it wasn't written by me or isn't my appropriately licensed intellectual property, please highlight that content and I am happy to discuss it. |
|
Thank you for your contribution to Klipper. Unfortunately, a reviewer has not assigned themselves to this GitHub Pull Request. All Pull Requests are reviewed before merging, and a reviewer will need to volunteer. Further information is available at: https://www.klipper3d.org/CONTRIBUTING.html There are some steps that you can take now:
Unfortunately, if a reviewer does not assign themselves to this GitHub Pull Request then it will be automatically closed. If this happens, then it is a good idea to move further discussion to the Klipper Discourse server. Reviewers can reach out on that forum to let you know if they are interested and when they are available. Best regards, PS: I'm just an automated script, not a human being. |
Add probe_scan MCU module that polls a GPIO pin at configurable intervals and timestamps state transitions. Events are buffered and streamed to the host via the bulk sensor subsystem. Excluded from limited-code-size builds (PRU) via Kconfig guard. Signed-off-by: Matti Airas <matti.airas@hatlabs.fi>
Add host-side module for continuous bed mesh scanning with binary inductive probes. Oscillates Z while traversing XY in serpentine or cross-hatch patterns, correlates MCU probe events with stepper history, and interpolates onto the bed_mesh grid using IDW. Signed-off-by: Matti Airas <matti.airas@hatlabs.fi>
Route BED_MESH_CALIBRATE METHOD=probe_scan to the probe_scan module for continuous-motion scanning instead of point-by-point probing. Signed-off-by: Matti Airas <matti.airas@hatlabs.fi>
Signed-off-by: Matti Airas <matti.airas@hatlabs.fi>
Replace custom ack/retransmit protocol with the standard sensor_bulk infrastructure. The old protocol exceeded the max message payload size and duplicated existing functionality. MCU side: embed struct sensor_bulk, buffer events in sb.data[], copy under IRQ protection and send with IRQs enabled. Add periodic flush countdown for sparse events and a status query command for overflow diagnostics. Host side: replace callback-driven ack protocol with pull-based BulkDataQueue. Add sequence gap detection. Kconfig: add WANT_PROBE_SCAN to NEED_SENSOR_BULK dependency. Signed-off-by: Matti Airas <matti.airas@hatlabs.fi>
e49cacb to
16c106f
Compare
|
I have performed a self-review following the review checklist: 1. Quality / defects
2. Contribution ImpactContinuous-motion bed mesh scanning for binary (on/off) inductive probes. Eliminates stop-and-go probing. A full mesh completes in ~30 seconds vs several minutes for point-by-point probing. Target audience: any Klipper user with a standard inductive probe (Super Pinda, Vinda, SupCR, Decoprobe, etc.). 3. Copyright / Signed-off-byAll new files have GPLv3 headers. All commits have 4. Coding conventionsFollows patterns from 5. Documentation updated
6. Commit Formatting5 atomic commits, each addressing a single topic with descriptive messages. Rebased on current master. |
|
Thanks. At a high-level, it seems like an interesting idea (obtain fast scanning for traditional inductive probes using a mechanism similar to how eddy current probes work). The big question for me is - is this a generally useful tool that lots of users will utilize across a wide variety of printers? (That is, does it have a "noteworthy target audience" as mentioned in point 2 of the code review notes at https://www.klipper3d.org/CONTRIBUTING.html#what-to-expect-in-a-review ). I'd like to be able to gauge if there is a lot of interest, if lots of people are willing to test, and if those testing results show good accuracy on a variety of printers. The Klipper Discourse may be a starting point for that ( https://www.klipper3d.org/Contact.html ). As for the use of AI, at a high-level I'd be inclined to follow what other major open-source projects do ( https://klipper.discourse.group/t/guidelines-for-code-obtained-with-help-of-ai-tools/25759/2 ). As an example, it seems the QEMU project has some concerns on copyrights (as highlighted earlier here). However, it seems other projects have a different approach. For what it is worth, though, I took a quick look at the code here and the AI generated code isn't great. It seems like a mash-up of the "buttons" code, "bulk_sensor" code, and "probe" code. And, although that's fine at a high-level, the generated code here clearly has "weird" vestiges of unrelated parts of those systems. Cheers, |
Summary
probe_scanmodule for continuous-motion bed mesh scanning with binary (on/off) inductive probes (NPN NC type: Super Pinda, Vinda, SupCR, Decoprobe, etc.)BED_MESH_CALIBRATE METHOD=probe_scan— no standalone commands neededDiscussion and background: https://klipper.discourse.group/t/continuous-mesh-probing-with-inductive-probe/25741
How it works
z_offset ± margin. The entire path is pre-queued to the motion planner for gap-free continuous motionTest plan
BED_MESH_CALIBRATE METHOD=probe_scanproduces a valid mesh (serpentine mode)BED_MESH_CALIBRATE METHOD=probe_scanproduces a valid mesh (cross-hatch mode)BED_MESH_CALIBRATEwithout METHOD still uses standard probingPROBE_SCAN_CALIBRATEstandalone hysteresis measurement worksZrefMode.PROBE🤖 Generated with Claude Code