Klipper3d / klipper

Klipper is a 3d-printer firmware
GNU General Public License v3.0
8.99k stars 5.17k forks source link

filament_switch_sensor: add option to always execute gcode handlers #6451

Open mdvorak opened 5 months ago

mdvorak commented 5 months ago

Added run_always config option as a follow-up to the #6435 @Arksine.

I'm open for better name for the variable.

Zeanon commented 5 months ago

maybe this is a dumb idea, but why not do something like this:

        if is_filament_present == self.filament_present and not force:
            return
        self.filament_present = is_filament_present
        eventtime = self.reactor.monotonic()
        if eventtime < self.min_event_systime:
            # do not process during the initialization time, duplicates,
            # during the event delay time, while an event is running, or
            # when the sensor is disabled
            return
        if is_filament_present:
            self.printer.send_event("filament:insert", eventtime, self.name)
        else:
            self.printer.send_event("filament:runout", eventtime, self.name)
        if not self.sensor_enabled:
            return

That would fire the event (in this case even when the sensor is disabled(reason being, a lot of people prefer to keep the sensors disabled cause for some odd reason the runout gcode sometimes tends to fire even though the printer is not actually in a printing state)) without changing any behaviour at all, except for firing the event

Arksine commented 5 months ago

That would fire the event (in this case even when the sensor is disabled(reason being, a lot of people prefer to keep the sensors disabled cause for some odd reason the runout gcode sometimes tends to fire even though the printer is not actually in a printing state)) without changing any behaviour at all, except for firing the event

The runout gcode cannot fire when idle_timeout reports Printing. The idle_timeout will report Printing if the printer is in any way busy, such as a stepper moving. Most likely some individuals confusing this with the print_stats printing state which is tied to the virtual_sdcard. The sensor has to keep compatibility with print sources over the pty, such as Octoprint, so we cannot depend on the latter. This is why its possible to enable/disable sensors via gcode.

Regardless, I don't think adding printer events should be a part of this PR, as they may block merging. The PR as-is looks good to me.

KevinOConnor commented 5 months ago

Thanks. We can certainly commit this change.

For what it is worth, though, I did not understand the documentation for this new parameter.

As a user, when would I set this new config option, and what real world tasks can I now achieve after setting it?

-Kevin

mdvorak commented 5 months ago

Most real-world examples are probably related to multi-material prints (generally, not only ERCF).

How can I improve the documentation?

github-actions[bot] commented 5 months ago

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:

  1. Perform a self-review of your Pull Request by following the steps at: https://www.klipper3d.org/CONTRIBUTING.html#what-to-expect-in-a-review If you have completed a self-review, be sure to state the results of that self-review explicitly in the Pull Request comments. A reviewer is more likely to participate if the bulk of a review has already been completed.
  2. Consider opening a topic on the Klipper Discourse server to discuss this work. The Discourse server is a good place to discuss development ideas and to engage users interested in testing. Reviewers are more likely to prioritize Pull Requests with an active community of users.
  3. Consider helping out reviewers by reviewing other Klipper Pull Requests. Taking the time to perform a careful and detailed review of others work is appreciated. Regular contributors are more likely to prioritize the contributions of other regular contributors.

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, ~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.

tphan26363 commented 3 months ago

The filament switch detection routine is sometime over detection. This happen even the switch only open in a very short time(us I guest) depend on how fast the MCU or the execution is. Vibration some time can open the switch even with the filament in in a very short time causing the false trigger. This especially happen to the bad quality switches. The routine should have to detect long pulse since the mechanical switch should open for a long time when the filament run out. 1 second long pulse has to be a minimum requirement for the detection to be valid. Howe ever this can also be fix mechanically just by revert the state of the switch when filament is present to " Open" instead of "Close". Since when filament in, any vibration can not close the switch no matter how violent it is. This require to revert the state of the pin definition and I found this to be extremely reliable solution. For the "Motion Sensor" I can not do much but I believe a complex "insert Gcode" when trigger can change the way of motion detect more reliable. The way motion detection right now with be not reliable. Just by printing at very slow speed can generate false triggering. I believe a more intelligent detection need to be implement. One thing to make sure that when the extruder stop, the motion sensor should not doing detection during the time extruder not moving the filament and when the extruder moves the filament, the detection should start working. Retraction should be ignore since the movement is to short for the motion sensor to feel it. Any way the current routine will work reliably if you are printing large object at a relative fast speed and that is the only case that motion sensor should be use for.