Marlin
Marlin copied to clipboard
MarlinFirmware
Implement M591 configurable runout sensors
Deprecates M412 and implements RRF compliant M591, matching functionality as previously defined on the reprap wiki.
Migrates existing M412 arguments to M591 with the addition of the Pnn parameter and Enn parameter. Add L parameter as alias to existing D parameter to maintain parity with RRF.
- Pnn Mode options as follows -- 0 = none, essentially disabled -- 1 = simple sensor (high signal when filament present) -- 2 = simple sensor (low signal when filament present) -- 7 = motion encoder sensor
Updated M119 to show filament present or missing along with motion if in motion mode.
Makes runout distance / debounce a standard function. Will need to evaluate impact on mixing extruders and potentially simply set the value to 0 when mixing is enabled.
Method for deprecation of M412 needs to be looked at as it currently does not appear to work as intended, it doesnt echo the message placed and tries to advance the gcode queue by 1, however it does process the function.
@marcio-cp as this deals with some of youre motion code, figured youd be a good one to review.
See RRF function here : https://duet3d.dozuki.com/Wiki/M591
This was tested on a couple machines here, but obviously not every possible combination.
Ill go about adding menus for this after at least the first round of review.
Can we do this with out adding any more #undef's ?
Those really bother some of the purist types....
The only one added is removing the temp function right above it. I'd have to look at why it was originally done in the endstop report function but I added the identical methodology to the pause resume assuming if it was necessary in endstops the same would apply as it's the same block to process.
Regardless, this is just a first round draft still so I'm sure stuff will change!
So if a user wants to change a simple filament sensor for a motion sensor, they will need to have configured both. But, it would be nice if we had an option to simply not compile in support for a motion sensor. And, it may also be useful to be able to have it statically configured with no runtime ability to change modes or sensor types, and only make this configurability available to users who opt into it.
The changes revising the encoder to type 3 break parity with rrf which has it defined as type 7, with intermediate types defined as specific duet hardware.
Thanks! I already noticed that, but hadn't updated yet due to falling asleep at my keyboard.
@thinkyhead ran this with 3 separate machines with different sensors swapping around and appears to work so far. Ill look into getting it onto an IDEX either later today or tomorrow for drastically different type test.
So if a user wants to change a simple filament sensor for a motion sensor, they will need to have configured both. But, it would be nice if we had an option to simply not compile in support for a motion sensor. And, it may also be useful to be able to have it statically configured with no runtime ability to change modes or sensor types, and only make this configurability available to users who opt into it.
Replying here for historical completion, as it was discussed on discord. A basic mode with configuration support for melzi or other tiny board may make sense, but with or without motion with runtime configuration doesn't save much unless they also lose de-bounce, as they essentially created the same variables to function. I dont believe the 3rd methodology has any real merit worth giving it attention.
SRAM consumption is +4b from the debounce method, +2b from motion, +11b from static no debounce Progmem +1444 from debounce, +1412 from motion, +2198 from static no debounce
1334b of that is just the menu addition, so from the most common setup (standard with debounce) if we put the new menu under no slim menus, we are adding 4b sram and 110b progmem to make it configurable. Probably less if we trim the endstop and M591 report output if slim menus is on as well. (just pushed that change)
Given that the melzi with the least progmem has double the ram of the 2560, im not concerned about the sram increase at all. With slim menu, the progmem addition is also negligible. Im not convinced the effort to support multiple variations here is worth the few bytes saved all things considered here.
We cant arbitrarily set pullup and pulldown based on the state. For one, I don't see that being a proper solution to any issue, as they have never required being forced to a specific state in the past with a polarity change. Not every hardware platform even supports both pullup and pulldown. Some only with 3.3v on one mode and 5v on the other. Most likely there was an electrical issue you were compensating for with it.
Even though its likely unnecessary, I added a call to runout.setup instead where runout.reset is likely the only required function, just so it can enforce the pin states if needed for some reason I cant see between changes.
I have a machine running mode 7 right now that was set via the menu, but it was flashed before the last rebase to head. I'll flash it again with the changes since the last rebase likely tomorrow.
Before this can be merged, it needs to restore the old filament runout handling, including retaining M412, and to make the M569 additions into optional features.
RRF is designed to be fully configurable at runtime so that you can add and remove hardware without a rebuild, and it has many G-codes to set pins for features. But Marlin requires a rebuild to change / add hardware. Marlin will continue to apply this microkernel (non-monolithic) philosophy.
Since there are apparently some setups with swappable filament sensors, filament sensors that operate in multiple modes, and multiple filament sensors that might be active and monitored at the same time, this is a welcome extension. But we need to be able to do a build for one or two basic filament sensors that is just as small as before. Maybe this can be done by simply altering the new code to use template classes, or by replacing direct references to variables with getter methods that become constexpr, creating never-used code paths that the compiler can strip out.
Originally M412 just output a deprecation warning then dropped into M591, ill see if that was lost somehow but that was present at one point.
Dropped a message on discord to make sure were on the same page on legacy support options with memory usage differences.