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

[FR] UART_ENDSTOPS for TMC2209 #24755

Open
cryptoAlgorithm opened this issue Sep 12, 2022 · 17 comments
Open

[FR] UART_ENDSTOPS for TMC2209 #24755

cryptoAlgorithm opened this issue Sep 12, 2022 · 17 comments
Labels

Comments

@cryptoAlgorithm
Copy link

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

@cryptoAlgorithm cryptoAlgorithm added the T: Feature Request Features requested by users. label Sep 12, 2022
@thisiskeithb
Copy link
Member

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
Copy link
Contributor

ellensp commented Sep 12, 2022

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

@cryptoAlgorithm
Copy link
Author

@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
Copy link
Contributor

phizev commented Sep 12, 2022

@ellensp when/if teemuatlut/TMCStepper#231 and something such as skruppy@c53622b 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
Copy link
Author

@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
Copy link
Author

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

@ellensp
Copy link
Contributor

ellensp commented Sep 13, 2022

anyone can create a PR,

@cryptoAlgorithm
Copy link
Author

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

@phizev
Copy link
Contributor

phizev commented Sep 13, 2022

@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 teemuatlut/TMCStepper#231 and skruppy@c53622b 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
Copy link
Author

@phizev I believe SPI_ENDSTOPS only polls the driver during homing

@thisiskeithb
Copy link
Member

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
Copy link
Contributor

phizev commented Sep 14, 2022

@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
Copy link
Author

@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
Copy link
Author

@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
Copy link
Member

@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
Copy link
Author

@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?

@thisiskeithb thisiskeithb changed the title [FR] SERIAL_ENDSTOPS for TMC2209 [FR] UART_ENDSTOPS for TMC2209 Sep 5, 2024
@fermino
Copy link

fermino commented Sep 19, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants