MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.19k stars 19.22k forks source link

[FR] UART_ENDSTOPS for TMC2209 #24755

Open cryptoAlgorithm opened 2 years ago

cryptoAlgorithm commented 2 years ago

Is your feature request related to a problem? Please describe.

Marlin has experimental support for SPI_ENDSTOPS, which negates the need for hardwiring a wire from DIAG0 to end stop pins. However, certain Trinamic drivers which also support StallGuard, such as the TMC2209, communicate thru serial and hence does not work with the SPI_ENDSTOPS feature.

Are you looking for hardware support?

Endstop polling thru Serial for Trinamic drivers that communicate thru serial and support StallGuard - e.g. TMC2209

Describe the feature you want

I'm proposing a feature similar to SPI_ENDSTOPS, but thru serial. It should require minimal changes, and an additional config flag (SERIAL_ENDSTOPS or similar). Polling the driver status thru Serial is very similar to polling thru SPI, and would require only very small changes to endstops.cpp. I can open a PR implementing the changes if need be.

Additional context

No response

thisiskeithb commented 2 years ago

an additional config flag (SERIAL_ENDSTOPS or similar)

UART_ENDSTOPS would probably be a better name for them since SPI_ENDSTOPS are for SPI-based drivers and TMC2209s are UART-based.

ellensp commented 2 years ago

uart to tmc2209 is software serial on most controllers. Ie really slow and cpu intensive... I think this is a really bad idea

cryptoAlgorithm commented 2 years ago

@ellensp how about if the sanity check throws a warning if you try to enable it on a non-hw serial based controller? Currently I'm doing testing on an esp32, so it has a dedicated hardware serial port (Serial2 in my case) for both controllers.

phizev commented 2 years ago

@ellensp when/if https://github.com/teemuatlut/TMCStepper/pull/231 and something such as https://github.com/skruppy/Marlin/commit/c53622b1a4f9aa285f70f11759a1cf8bc344d781 land, it'll allow a path to avoid the hard dependency on software serial on platforms that can do it in hardware.

I have a RAMPS board running quite happily with 4 TMC2209's on hardware UART with: ENDSTOP_INTERRUPTS_FEATURE (Incompatible with the software serial library.) MONITOR_DRIVER_STATUS (Horribly slow with software serial when forced to compile.) TMC_DEBUG Software serial disabling interrupts also wrecks my Z probe repeatability. (Still usable, but an order of magnitude worse.)

So while it is presently software on many controllers, I think it's a really unfortunate code shortcoming more than anything else.

@cryptoAlgorithm presently the TMC library has a dependency on the software serial library. It stops some platforms from using hardware serial even when it's available. The best summary/solution of the issue I have found so far is in the PR above.

cryptoAlgorithm commented 2 years ago

@phizev, great to hear from your experience. I'm currently in the prototype stage with 2 TMC2209s hooked up to nema17 motors and an esp32 on a dev board. Currently im using a haphazard method of hooking up a jumper from the DIAG pin of the jumpers to the end stop pins on the esp32. My config is exactly the same as yours, and as far as I can test, sensorless homing works reliably, albeit with the need of a jumper.

Regarding the dependency on the software serial library, wouldn't features such as switching from spreadCycle to stealthChop require the TMC library as well? Doesn't that mean it already stops some platforms from using hw serial for that functionality? If UART communication is already enabled, the TMC library is already used, so using the library to poll the DIAG status thru UART wouldn't incur further functionality loss, no?

cryptoAlgorithm commented 2 years ago

If I'm given the go-ahead, I could try drafting a PR, too

ellensp commented 2 years ago

anyone can create a PR,

cryptoAlgorithm commented 2 years ago

@ellensp Yes, but I wouldn't like spending time on a feature thats flat out not going to be accepted

phizev commented 2 years ago

@cryptoAlgorithm

Regarding the dependency on the software serial library, wouldn't features such as switching from spreadCycle to stealthChop require the TMC library as well? Doesn't that mean it already stops some platforms from using hw serial for that functionality?

Yep, that's the exact reason for https://github.com/teemuatlut/TMCStepper/pull/231 and https://github.com/skruppy/Marlin/commit/c53622b1a4f9aa285f70f11759a1cf8bc344d781 so that more platforms can use hardware serial instead of software.

If UART communication is already enabled, the TMC library is already used, so using the library to poll the DIAG status thru UART wouldn't incur further functionality loss, no?

Using software serial has a high overhead, it's not that functionality is lost. Marlin cannot be compiled with MONITOR_DRIVER_STATUS for that reason.

Unfortunately, I don't think there is anything we can do to speed along an update to the TMC library to allow more platforms to use hardware serial with the TMC drivers. I've found using hardware serial to be far more reliable with TMC 2209's as well.

@ellensp would using software serial for stallguard endstops only have an effect while homing by default? Is it common to have endstops always enabled while printing?

cryptoAlgorithm commented 2 years ago

@phizev I believe SPI_ENDSTOPS only polls the driver during homing

thisiskeithb commented 2 years ago

I haven’t checked the code to see if it’s compatible with SPI_ENDSTOPS, but users can also enable ENDSTOPS_ALWAYS_ON_DEFAULT which keeps endstops active all the time.

phizev commented 2 years ago

@thisiskeithb the software serial library used by the TMC library is incompatible with ENDSTOP_INTERRUPTS. This means Marlin has to constantly poll when ENDSTOPS_ALWAYS_ON_DEFAULT is used with TMC2209's on AVR, and likely other platforms. I almost abandoned using TMC serial drivers due to this as the performance tanked, and I think having endstops always enabled is a safety necessity.

As it stands, TMC2209 and AVR (and other affected platforms which can't brute force with processing power) have the following issues due to unnecessarily forcing software serial:

  1. ENDSTOPS_ALWAYS_ON_DEFAULT is unusable. (ENDSTOP_INTERRUPTS unavailable with software serial.)
  2. MONITOR_DRIVER_STATUS is unusable due to performance overhead.

So when using the TMC library, we can only use endstops while homing on some boards.

@ellensp would polling with software serial only while homing be an issue? If it isn't, then software serial for endstops would match the current feature set. If it is still a problem, then both situations can be resolved with the "fixed" TMC library allowing the use of hardware serial.

cryptoAlgorithm commented 2 years ago

@hisiskeithb the doc comment right above the SPI_ENDSTOPS flag has this to say: (emphasis my own)

Poll the driver through SPI to determine load when homing.

cryptoAlgorithm commented 2 years ago

@phizev I would suppose adding such a feature would make most sense if it was only enabled during homing, if performance while using software serial is as bad as you've described. I suppose, throwing warnings or even errors in the sanity check if this feature is enabled and sw serial is used would be an acceptable mitigation, so the user is well aware that this would very likely tank performance. I believe, with more manufacturers moving to ARM-based boards, this would be less of an issue in the future, hence adding it as a feature now would make it more convenient for future users down the road.

thisiskeithb commented 2 years ago

@hisiskeithb the doc comment right above the SPI_ENDSTOPS flag has this to say: (emphasis my own)

Poll the driver through SPI to determine load when homing.

Yes, but you can still enable ENDSTOPS_ALWAYS_ON_DEFAULT.

cryptoAlgorithm commented 2 years ago

@thisiskeithb I'm not sure enabling ENDSTOPS_ALWAYS_ON_DEFAULT and SPI_ENDSTOPS would cause the driver to be constantly polled thru SPI while printing, since that would surely have an impact if printing from SD, right?

fermino commented 1 week ago

My two cents: I think this would be useful for boards with sufficient capabilities (I'm for example using a board with 4 TMC2209 with HW serial, and having the endstop pins available for other things would be a win. I'm also using 115200 baud/s so for X/Y homing on cartesian printers the small delay shouldn't really be an issue.