Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Filament Run-out Sensor functionality broken / limited #20224

Closed
Misterke opened this issue Nov 20, 2020 · 4 comments
Closed

[BUG] Filament Run-out Sensor functionality broken / limited #20224

Misterke opened this issue Nov 20, 2020 · 4 comments

Comments

@Misterke
Copy link
Contributor

Bug Description

Merged pull-request 19965 (so far bugfix-2.0.x only) adds support for multiple run-out sensors, but at the same time breaks support for run-out sensors that open when filament runs out (i.e. where the pull-up/down value is attained when filament is out. This is a consequence of no longer having separate FIL_RUNOUT_PULLUP and FIL_RUNOUT_PULLDOWN settings, but combining them into a single FIL_RUNOUT_PULL and using as the direction for the pulling the inverse of the FIL_RUNOUT_STATE value.

Configuration Files

I cannot supply a working config as the above makes it impossible for me to create a working config with the current macros. I would want to use in the config something like:

#define FILAMENT_RUNOUT_SENSOR
#if ENABLED(FILAMENT_RUNOUT_SENSOR)
  #define FIL_RUNOUT_ENABLED_DEFAULT true // Enable the sensor on startup. Override with M412 followed by M500.
  #define NUM_RUNOUT_SENSORS   1          // Number of sensors, up to one per extruder. Define a FIL_RUNOUT#_PIN for each.
  #define FIL_RUNOUT_STATE     HIGH       // Pin state indicating that filament is NOT present.
  #define FIL_RUNOUT_PULLUP    1         // Use internal pullup / pulldown for filament runout pins.

but FIL_RUNOUT_PULLUP no longer exists and setting FIL_RUNOUT_PULL will just cause the pin to be pulled down instead of up!

Steps to Reproduce

Just try configuring for a run-out sensor that connects the run-out pin to GND when filament is present and breaks the connection when filament runs out ...

Expected behavior:

Configuration should allow specifying the pull-up/down direction independent from the pin-state that would flag run-out.

Actual behavior:

Configuration does not allow independent FIL_RUNOUT_STATE and FIL_RUNOUT_PULLUP or FIL_RUNOUT_PULLDOWN direction.

Additional Information

Problem caused by pull-request (#19965). Remark also given there, but the pull-request is already merged. See the comment (#19965 (comment)).

@thinkyhead
Copy link
Member

I see. I guess it makes a certain sense since we're detecting, not the runout state, but the presence state. It may actually be that the real problem is that the pull is inverted from what it should be, due to this reversed meaning.

@Misterke
Copy link
Contributor Author

This (just inverting the pull-up/down) will not fix the problem, it just changes which filament run-out sensors are supported and which are not.

Take the example of a sensor with a simple end-stop switch which has 3 pins: Common, Normally-Open and Normally-Closed. If you connect the Common pin to GND, then depending on which other pin of the switch you use (NO or NC), your STATE value will have to be either HIGH (for NO) or LOW (for NC), but the actual GPIO always has to be pull-up as the other side is connected to GND. As there are numerous forms of run-out sensors possible, it is just limiting functionality to link pull-up/down direction to the STATE.

@thisiskeithb
Copy link
Member

Closing since #20242 was merged.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Jan 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants