hall_filament_width_sensor: Add option to enable or disable flow compensation, add command descriptions, and improve query command output

[benlye]

This PR proposes several changes to the hall_filament_width_sensor.

The primary change is to add a configuration option to enable or disable the flow compensation element of the hall_filament_width_sensor.

Some printers, including several models from Qidi such as the Q1 Pro and Plus4, have a hall effect sensor which is too inaccurate to helpfully adjust the extrusion multiplier, but is good enough to act as a filament runout sensor. By having the option to enable the width sensor but disable flow compensation, the width sensor can still be used as a runout sensor.

The PR adds a new config option enable_flow_compensation to hall_filament_width_sensor which enables this choice. The option defaults to True, ensuring that the existing default behavior is not affected.

Three new commands are also added, which can be used to enable or disable flow compensation and check the current state:

• ENABLE_FILAMENT_WIDTH_COMPENSATION: Turn on flow compensation based on filament width sensor readings.

• DISABLE_FILAMENT_WIDTH_COMPENSATION: Turn off flow compensation based on filament width sensor readings. Does not disable the sensor itself or the filament runout switch.

• QUERY_FILAMENT_WIDTH_COMPENSATION: Report the current state of flow compensation.

Note that while flow compensation can also be disabled by setting max_difference to 0, this forces the extrusion multiplier to 100% and prevents the user from changing it to any other value.

Additional changes:

• Missing command descriptions are added to the existing hall filament sensor commands
• The output of the existing QUERY commands is improved:
  • QUERY_FILAMENT_WIDTH output is limited to four decimal places
  • An extra space is removed from the output of QUERY_RAW_FILAMENT_WIDTH

Current query command outputs:

QUERY_FILAMENT_WIDTH
Filament dia (measured mm): 1.67484578323

QUERY_RAW_FILAMENT_WIDTH
ADC1= 5008 ADC2=9580 RAW=14588

New query command outputs

QUERY_FILAMENT_WIDTH
Filament dia (measured mm): 1.6748

QUERY_RAW_FILAMENT_WIDTH
ADC1=5008 ADC2=9580 RAW=14588

The documentation is updated to reflect the changes.

Signed-off-by: Ben Lye ben@lye.co.nz

[dewi-ny-je]

Let's not limit the scope to said printers: I added a reliable Hall sensor to mine but I don't want it enabled when I use soft filaments, while I still need the runout switch to operate!

I totally approve the idea behind this change.

[benlye]

FWIW, I have tested this code on my Qidi Q1 Pro printer and can confirm:

• The enable_flow_compensation config setting works as expected in both states to set the initial / default behavior
• The current sensor behavior is preserved if the setting is not present (width compensation is enabled)
• ENABLE_FILAMENT_WIDTH_COMPENSATION enables flow compensation
• DISABLE_FILAMENT_WIDTH_COMPENSATION disables flow compensation
• The filament runout switch triggers correctly when enable_flow_compensation is in either state
• Manual extrusion factor changes work correctly when enable_flow_compensation is false

[dewi-ny-je]

I am not a coder, but I can understand code to some degree and the topic.
All I can do is ask GPT5.2 thinking, which raised the following issues:

• as you stated, the multiplier is not reset to 100% upon disabling compensation. Actually it should probably go back to 100%, unless a more complex logic of saving the initial state is implemented (which I don't think it's needed).
• If compensation was turned off while M221 was not 100, and filament later becomes “absent”, you won’t get the old safety reset back to 100. So, consider always resetting to M221 S100 on “filament absent” regardless of the compensation flag. In cmd_flow_comp_disable(), also do: self.gcode.run_script_from_command("M221 S100") for example
• No fallback M221 S100 when diameter is out of bounds (if compensation disabled). Old behavior: if measured width went outside [min_diameter, max_diameter], it explicitly forced M221 S100. New behavior: that fallback only happens if enable_flow_compensation is True. So, if compensation is disabled and M221 was previously set to a compensated value, an out-of-range reading will no longer force “neutral” flow.
• “enable_flow_compensation” state isn’t exposed in get_status(). Add 'flow_compensation_enabled': self.enable_flow_compensation
• disabling sensor still resets M221 even if compensation is disabled: sometimes M221 is touched even when compensation is “off”, sometimes it isn’t. So, decide on the intended bnehaviour: if “compensation disabled” means “never touch M221”, then you may want to gate those resets too. If it means “don’t compute dynamic flow, but safety resets are OK”, then keep resets unconditional—but also apply the same philosophy to the “filament absent” safety path.

[benlye]

• as you stated, the multiplier is not reset to 100% upon disabling compensation. Actually it should probably go back to 100%, unless a more complex logic of saving the initial state is implemented (which I don't think it's needed).
• If compensation was turned off while M221 was not 100, and filament later becomes “absent”, you won’t get the old safety reset back to 100. So, consider always resetting to M221 S100 on “filament absent” regardless of the compensation flag. In cmd_flow_comp_disable(), also do: self.gcode.run_script_from_command("M221 S100") for example
• No fallback M221 S100 when diameter is out of bounds (if compensation disabled). Old behavior: if measured width went outside [min_diameter, max_diameter], it explicitly forced M221 S100. New behavior: that fallback only happens if enable_flow_compensation is True. So, if compensation is disabled and M221 was previously set to a compensated value, an out-of-range reading will no longer force “neutral” flow.

I was on the fence with resetting to 100%. I was trying to modify as little as possible but actually I agree with you. I have added M221 S100 when compensation is disabled, which I believe addresses all three points.

• “enable_flow_compensation” state isn’t exposed in get_status(). Add 'flow_compensation_enabled': self.enable_flow_compensation

I've added flow_compensation_enabled to the status report and updated the status reference doc accordingly. I also added the QUERY_FILAMENT_WIDTH_COMPENSATION command so the user can easily get the current state.

• disabling sensor still resets M221 even if compensation is disabled: sometimes M221 is touched even when compensation is “off”, sometimes it isn’t. So, decide on the intended bnehaviour: if “compensation disabled” means “never touch M221”, then you may want to gate those resets too. If it means “don’t compute dynamic flow, but safety resets are OK”, then keep resets unconditional—but also apply the same philosophy to the “filament absent” safety path.

I believe that this is now consistent as the flow will be reset to 100% if flow compensation is disabled. I have updated the documentation to explicitly state when the flow rate is reset to 100%.

While I was doing this I also added some command help for all the commands (not just the ones I added) and improved the formatting the output of the QUERY_ commands.

[MRX8024]

Also look at CONTRIBUTING.md regarding the formation of commits.

[benlye]

Also look at CONTRIBUTING.md regarding the formation of commits.

Thanks for the feedback (and your time). I have reorganized the changes into three distinct commits. Hopefully this is better?

I also updated the PR description to better reflect all of the proposed changes.

[MRX8024]

You have to add "Signed-off-by" to every commit (doc). Otherwise everything is probably fine.

[benlye]

You have to add "Signed-off-by" to every commit (doc). Otherwise everything is probably fine.

All done. Thanks again for your feedback.

[Phil1988]

I can not verify this is working for me on the Plus4 and I didnt found any issues at this hall_filament_width_sensor.py 👍

[Phil1988]

Properly coded and documentation added - looking good to me and could be merged

[dewi-ny-je]

Some remarks

Disabling compensation does not clear queued diameter readings

cmd_flow_comp_disable() resets M221 S100, but does not clear filament_array.

This allows to briefly disable and then re-enable and use already-queued values, but re-enabling later could immediately apply an “old” diameter value. Maybe just clear up the queue and that's it, or let's think to a use case where the user disabled the compensation, and re-enables it without touching the filament in between. I can't find any.

Division by zero

There’s no guard for calibration division-by-zero: (self.rawdia2 - self.rawdia1) could be zero if misconfigured, leading to a crash in adc2_callback(). It was there also in the original module.

Indentation

The update_filament_array() block uses inconsistent indentation for several lines—e.g. next_reading_position = ..., the following if, and the append(...) are indented differently than the surrounding code (they don’t line up with the usual 4-spaces-per-level style).

[benlye]

Disabling compensation does not clear queued diameter readings

cmd_flow_comp_disable() resets M221 S100, but does not clear filament_array.

This allows to briefly disable and then re-enable and use already-queued values, but re-enabling later could immediately apply an “old” diameter value. Maybe just clear up the queue and that's it, or let's think to a use case where the user disabled the compensation, and re-enables it without touching the filament in between. I can't find any.

I don't think this is a problem. We're only disabling compensation - the filament width readings are still being taken while compensation is disabled, so the queue will contain current values to be used if/when compensation is re-enabled. Disabling the sensor does clear the readings.

Division by zero

There’s no guard for calibration division-by-zero: (self.rawdia2 - self.rawdia1) could be zero if misconfigured, leading to a crash in adc2_callback(). It was there also in the original module.

I've added a new commit to eliminate that and another potential div-by-zero in extrude_factor_update_event by checking that configuration values can't lead to those conditions.

Indentation

The update_filament_array() block uses inconsistent indentation for several lines—e.g. next_reading_position = ..., the following if, and the append(...) are indented differently than the surrounding code (they don’t line up with the usual 4-spaces-per-level style).

There's a few spacing and indentation issues throughout the module. The contributing guidelines say:

"Whitespace changes should not be mixed with functional changes. In general, gratuitous whitespace changes are not accepted unless they are from the established "owner" of the code being modified."

So I'm going to leave that as-is unless I get direction to clean it up.

[dewi-ny-je]

Perfect then, no further comments on my side.

[KevinOConnor]

Thanks. In general it seems fine to me. I'd ask for a change to the g-code commands though.

This module is an "old school" module in that it defines a lot of g-code commands. For the last several years, however, we've been reluctant to add lots of new G-Code commands. The issue is, each time we add a new command it risks conflicting with other modules and user defined macros.

This PR adds 3 new commands. Would it be possible to just add new parameters to the existing commands? For example, instead of adding a new QUERY_FILAMENT_WIDTH_COMPENSATION could the info just be displayed in the existing QUERY_FILAMENT_WIDTH command? Instead of addding new ENABLE/DISABLE_FILAMENT_WIDTH_COMPENSATION commands, could a new parameter "FLOW_COMPENSATION=[0|1]" be added to the existing ENABLE_FILAMENT_WIDTH_SENSOR command?

Cheers,
-Kevin

[benlye]

This module is an "old school" module in that it defines a lot of g-code commands. For the last several years, however, we've been reluctant to add lots of new G-Code commands. The issue is, each time we add a new command it risks conflicting with other modules and user defined macros.

Ah, I see. Thanks for the explanation. I have refactored the changes to use the existing commands instead of adding new ones, and made some other improvements:

• Fixed a bug where the filament sensor state could be out of sync with the width sensor state, leading to the filament sensor being stuck in the state it was in when the width sensor was disabled and the UI being out of sync with the printer's state
• Fixed a race condition when the sensor was being disabled and the M221 S100 reset commands were ignored

I've force-pushed to the PR branch. If it's better to close this PR and open a new one for a fresh review, please let me know.