[1.1.x] TMC driver update #8712

teemuatlut · 2017-12-08T22:09:26Z

New gcode commands:

M122: TMC debugging command. Pulls several registers from all the configured drivers and parses the bits and values.
M915: Calibrate Z axis. Basically a script that lowers Z driver current and drives the axis past the configured maximum height. Then home Z again because we intentionally lost some steps. This is quite a new feature still and I haven't used it much (because it's a calibration command). I would've also preferred to give it a G command but didn't know which one to go for. I don't know if Prusa printers have a dedicated gcode command or if it's LCD only.

Features:

Support for TMC2208 drivers in HW and SW serial modes.
Automatic current control deprecated and replaced with a driver monitoring system. The new system polls the drivers twice a second and lowers the current if overtemperature prewarn is triggered. Can also stop the printer if a driver is in an error state (short to ground or overtemperature). S0/1 argument reports the "actual current scale" of the driver.
Store sensorless homing setting into EEPROM.

Default configuration:

Tune some blank time and hysterisis settings.
Lower default current values. Shouldn't expect everyone to have cooling fans and dual heatsinks for drivers.

This is actually a much bigger update that I'm comfortable with and I feel it might be a bit error prone. But I have been using this fork for quite a while now and it should work ok. Mostly I wanted to stop telling people to use my fork to solve their problems.

thinkyhead · 2017-12-08T22:48:05Z

This is epic, and looks good overall. Can we make a PR for the 2.0 branch to match? I might hold off on merging this till after the 1.1.7 release, since that is long overdue. (The 1.1.8 release will be more on-time, probably around 30 days after 1.1.7.)

teemuatlut · 2017-12-08T23:20:21Z

I've got a 2.0 based branch ready but I'll go over it tomorrow.
That said, I haven't yet used the v2.0 Marlin on my main printer and the 2.0 PR will be mostly theoretical. I will be moving all my work on v2.0 after this however.

thinkyhead · 2017-12-09T03:34:15Z

@teemuatlut Sounds good to me! Theoretically, the 1.1.7 release could be the last release that gets new features, and we could move all development to 2.0.x starting this week. The 1.1.x branch has only been waiting for a point where 2.0.x could build reliably for AVR and DUE, and though the jury is still out, I think it's acceptable if some components of the 2.0 official release are labeled as still being in beta, or even alpha….

Leaving 1.1.x behind should help to streamline all our development efforts.

teemuatlut · 2017-12-09T08:28:25Z

I'm going to revert your changes regarding the macros. Otherwise we're going to end up with a situation where we have #if ENABLED(HAVE_TMC2130) || ENABLED(HAVE_TMC2208) || ENABLED(HAVE_TMC2224) || ENABLED(HAVE_TMC5130) || ENABLED(HAVE_TMC2660)

thinkyhead · 2017-12-11T04:17:55Z

Why the increment to the EEPROM version?

thinkyhead · 2017-12-11T04:32:19Z

I'm going to revert your changes regarding the macros. Otherwise…

The main general-purpose macros.h file is a poor place for TMC-specific macros. If we need to refer to these macros in Configuration_adv.h, I suggest we put them in a separate tmc_macros.h file and include that in MarlinConfig.h before Configuration_adv.h instead.

Another option would be to create a separate Configuration_Steppers.h file and move all the alternative stepper options into it, along with the macros. It would be kind of unprecedented to add a new config file, but the number of options related to these steppers is starting to dominate Configuration_adv.h.

teemuatlut · 2017-12-11T07:42:57Z

the number of options related to these steppers is starting to dominate Configuration_adv.h

😄
Maybe we should actually consider making some logical dividing of the settings in general (perhaps for v2.0). At this time there are quite a lot of settings both in configuration.h and configuration_adv.h and both are over 1500 lines long. It can take a while to find the right setting if you don't know the correct keyword to search for.

I moved the tmc macros to their own file. It looks a bit silly as there's nothing else in there but maybe there will be use for it in the future as well. I know TRAMS is going to add a lot of things.

Why the increment to the EEPROM version?

I've added EEPROM support for the SENSORLESS_HOMING sensitivity values.

fiveangle · 2017-12-12T18:10:18Z

the 1.1.7 release could be the last release that gets new features

❤️ ❤️ ❤️

Another option would be to create a separate Configuration_Steppers.h file and move all the alternative stepper options into it, along with the macros. It would be kind of unprecedented to add a new config file, but the number of options related to these steppers is starting to dominate Configuration_adv.h.

This 💯 %

-=dave

thinkyhead · 2017-12-13T00:36:26Z

Maybe we should actually consider making some logical dividing of the settings in general (perhaps for v2.0)

At this point I would only break out settings that are rare, so that we don't have to duplicate them in every example configuration. Also there's the issue of having settings at hand. Even breaking out a Configuration_LCD.h file starts to make it harder to find a particular setting that you want to tweak. And, I will never stop repeating what a hassle it is to maintain just 2 configuration files across all the examples, let alone 3 or 4… or 10…

teemuatlut · 2017-12-13T23:16:49Z

I'd actually argue the other way that because the configuration files are getting so massive I find it harder to find a particular setting.

How do you approach updating all the config files? The way I've done it for these two PRs is using the replace method in sublime text. A regex like // @section TMC2130[\s\S]+#endif // TMC2130 would find the whole section in all the files and I'd use the new updated section as the string that replaces it. If you know the number of _adv files you can get an idea of whether or not it found all of them.

p3p · 2017-12-13T23:49:17Z

The best solution I could think of to the example config issue was to have a json (or any other preferred structure) of the settings required for each example, then regenerate the example config files from the current default config. It actually worked quite well in my python proof of concept, (I generated the diffs from the current examples) .. the obvious extension of this was to always generate the default config from a json data structure too and use that as the master.. which would be preferred method.

teemuatlut · 2017-12-13T23:58:03Z

Maybe the example configuration sets can be handled by the configurator that may be coming in the far far future.

thinkyhead · 2017-12-15T21:01:08Z

I'd actually argue the other way that because the configuration files are getting so massive I find it harder to find a particular setting.

I hope most users will use Find in their text editor. Of course, the first time configuring you may scan the first file, reading each option in turn, but once you've done a config or two, you just search for the options you want. For more obscure features, we provide web pages and point out the advanced options there, so users who want to know about them can Google for them. Or, enterprising users can search Configuration_adv.h for their keyword and probably find it.

The debate over whether to split up the configurations further has been had before, and the consensus that developed last time was that adding more files simply adds more headaches, while two files (our minimum) are relatively easy to maintain and keep in sync.

A "configurator" can hide most options and then array the rest in 2D space. A Configurator can also include presets and other helpful shortcuts and cues about which options are relevant or important. A configurator can include audio-visual guidance. The configurator by @akaJes is pretty much what we would like to have as a web-based utility, but unfortunately it was written with total dependency on Windows.

… into bugfix-1.1.x-HD-CR10 * 'bugfix-1.1.x' of https://github.com/MarlinFirmware/Marlin: [1.1.x] TMC driver update (MarlinFirmware#8712) Correct unskew, after all Missing HAS_HEAT_BED conditional

byebyedizzy · 2017-12-16T07:15:12Z

I wonder if it's possible to add support for TMC2208 when using single shared UART interface and analog commutator like 77hc4066 (scheme can be taken from 2208 datasheet) ? The main concern is that there is no SoftwareSerial library for some boards, for example Arduino Due. And amount of available hardware serials is rather limited (4 total for Due). This will require, as I understand, some sort of CS pin (for use with commutator) along with shared RX/TX pins.

teemuatlut · 2017-12-16T21:14:06Z

@thinkyhead Was this PR completely left out on the v1.1.7 release notes?

teemuatlut · 2017-12-16T21:15:21Z

@byebyedizzy I'd actually rather see the software serial library improved with support for DUE.

* origin/1.1.x: (346 commits) Version 1.1.7 Update README.md for bugfix Adding support for the Tronxy and Zonestar LCD Apply ZONESTAR_LCD to example configs More externs and functions in Marlin.h Add NANODLP_ALL_AXIS to config examples Extended NanoDLP_Z_Move_Sync feature to G4 and G28, Added optional move_sync for all axis. [1.1.x] TMC driver update (MarlinFirmware#8712) Correct unskew, after all Missing HAS_HEAT_BED conditional Reduce code with no heated bed General code/bitmaps cleanup Fix Planner::unskew parity with skew Comment, improve filament width sensor Separate Dual X un-park from movement Skew should apply per-segment Extend Skew Correction to UBL Travis Test for non-segmented UBL One or the other? UBL devel debugging flag ...

byebyedizzy · 2017-12-17T11:39:02Z

Well, I could not find any suitable replacement. Actually there are too few software serial implementations for Due. One that I found (https://github.com/antodom/soft_uart/blob/master/README.md) has too much limitations to be used in Marlin code. So shared UART with commutator is rather attractive - simple schematic and (I hope) not too complex to implement ...

thinkyhead · 2017-12-25T09:57:58Z

@teemuatlut It appears that some of this PR and #8769 are out of sync, especially configuration_store.cpp. Also the code that was added to configuration_store.cpp is incomplete. It will only compile with HAVE_TMC2130 enabled. See line 2063 in the 1.1.x version of that file for one error. It seems that it should be #if ENABLED(HAVE_TMC2130) || ENABLED(HAVE_TMC2208).

teemuatlut · 2017-12-25T10:21:26Z

Both 1.1.x and 2.0.x should be HAS_TRINAMIC
Other than that there's no really nothing much else in the configuration_store.cpp. The bit position calculations are purely cosmetic and ofc need to be updated but that doesn't affect functionality.

Did you find something else as well?

thinkyhead · 2017-12-25T10:39:00Z

Nope. Everything else is perrrrrrfectly ok. e0d487e

jmdearras · 2018-01-19T17:00:00Z

why was automatic current control removed?

teemuatlut · 2018-01-19T17:40:03Z

It didn't work as intended so I reworked it into current state but kept the current step down feature.

jmdearras · 2018-01-19T17:55:47Z

My son just had got a printer halt due to overtemp with that feature turned on. He's going to debug it this evening.

bleem313 · 2018-01-20T04:20:31Z

Hey, My Y driver is acting weird. It seems to be moving as if it is set for 8 micro steps instead of 16. When I pull the debug info, the Y has a different vsense. Do you have any ideas what might be causing this and how to fix it?

M122
X Y Z E0
Enabled false false false false
Set current 900 1000 800 800
RMS current 887 994 795 795
MAX current 1251 1402 1121 1121
Run current 28/31 17/31 25/31 25/31
Hold current 7/31 4/31 6/31 6/31
CS actual 7/31 4/31 6/31 6/31
PWM scale 42 57 33 1
vsense 1=.18 0=.325 1=.18 1=.18
stealthChop true true true true
msteps 16 16 16 16
tstep 1048575 1048575 1048575 1048575
pwm
threshold 0 0 0 0
[mm/s] - - - -
OT prewarn false false false false
OT prewarn has
been triggered false false false false
off time 5 5 5 5
blank time 24 24 24 24
hysterisis
-end 2 2 2 2
-start 3 3 3 3
Stallguard thrs 0 0 0 0
DRVSTATUS X Y Z E0
stallguard
sg_result 0 0 0 0
fsactive
stst X X X X
olb
ola
s2gb
s2ga
otpw
ot
Driver registers:
X = 0x80:07:00:00
Y = 0x80:04:00:00
Z = 0x80:06:00:00
E0 = 0x80:06:00:00

ok

teemuatlut · 2018-01-20T08:00:55Z

Your microsteps are reported correctly so I can see three possibilities.

Wrong steps per mm setting in Marlin. Check with M503.
For some reason the MCU sends too many pulses. Need an oscilloscope or a logic analyzer for this one.
double edged stepping is turned on. Needs added code to see the register value.

#9230 Perhaps this issue is related?

Y has different vsense bit because of your current setting.

thinkyhead · 2018-01-22T01:08:05Z

Users on the "3Dtechtalk" Facebook group are wondering about Automatic Current Control, and whether MONITOR_DRIVER_STATUS is an adequate replacement.

Can we provide some guidance?

thinkyhead · 2018-01-22T01:24:58Z

Another question arises. When using TMC2130 steppers, I assume it is required to define X_CS_PIN, Y_CS_PIN, etc., for each TMC2130-based stepper driver? If so, then I should add a proper sanity check.

bleem313 · 2018-01-22T05:53:59Z

Another question arises. When using TMC2130 steppers, I assume it is required to define X_CS_PIN, Y_CS_PIN, etc., for each TMC2130-based stepper driver? If so, then I should add a proper sanity check.

I know that at least when using any Ramps based boards, those pin are already defined in the pin_ramps.h file. So maybe changing it so they are defined in the config file and adding a sanity check would be a good thing, as I know I have to redefine them to run the SD card at the same time on my setup.

teemuatlut · 2018-01-22T08:05:32Z

@thinkyhead We have the predefined default pins in pins_ramps.h.

Sean is correct that Automatic increased the stepper current until OTPW was triggered and then back down a bit.
Monitoring will just tune down the current if the OTPW is triggered. It will also react to other error conditions like short to ground and open load. By Trinamic's design Short to ground will turn off the driver which of course ruins the print as the other steppers will happily continue moving. The monitoring feature can then abort the print so material is not wasted.

When I get to it, I'll add support for coolStep which further improves the current management capabilities making the stepper current react to load and not just temperature.

thinkyhead · 2018-01-22T09:45:51Z

We have the predefined default pins in pins_ramps.h.

Right, so sanity check would handle the case where TMC2130 is selected with non-RAMPS.

teemuatlut · 2018-01-22T09:47:47Z

Well the compiler would tell you that much but I guess having a sanity check would be more descriptive for the end user.

thinkyhead · 2018-01-22T09:48:39Z

Yes. The aim of sanity check is to catch what would otherwise be indecipherable compiler errors.

changing it so they are defined in the config file

A PR is in to allow them to be overridable. This is probably not an uncommon need.

teemuatlut · 2018-01-22T10:07:09Z

The CS pins likely need to be moved to AUX2 as the X_CS is also SDSS and Y_CS is SD_DETECT_PIN. The current defaults are from when Marlin originally used the other TMC library.

thinkyhead · 2018-03-18T22:10:34Z

Marlin/Marlin_main.cpp

+
+    #if ENABLED(HAVE_TMC2208)
+      delay(100);
+      tmc2208_init();


@teemuatlut — Re-init not needed in M80?

thinkyhead force-pushed the TMC_update_PR branch from c36539c to 429e2d3 Compare December 8, 2017 22:59

thinkyhead added PR: Improvement C: Boards/Pins labels Dec 9, 2017

teemuatlut and others added 2 commits December 9, 2017 01:08

Updates to TMC support, adding TMC2208

30af0f8

Fix PIN_EXISTS usage

ff15f70

thinkyhead force-pushed the TMC_update_PR branch from 88e1629 to 13efb69 Compare December 9, 2017 07:09

thinkyhead added 2 commits December 9, 2017 01:13

Move TMC macros to Conditionals_post.h

75b95cf

Keep this, if possible

736bfd1

thinkyhead force-pushed the TMC_update_PR branch from 13efb69 to 736bfd1 Compare December 9, 2017 07:13

Increment version tick

876ec1a

thinkyhead force-pushed the bugfix-1.1.x branch 2 times, most recently from 38898fa to 7c1adff Compare December 11, 2017 09:13

Move TMC macros to new file

c6ce0cb

teemuatlut mentioned this pull request Dec 12, 2017

[2.0.x] TMC driver update #8769

Merged

thinkyhead and others added 3 commits December 13, 2017 00:32

Update configuration_store.cpp

f5ca82f

Fix E0_SERIAL_TX_PIN

454845b

AUTOMATIC_CURRENT_CONTROL is replaced by MONITOR_DRIVER_STATUS

3a5084a

teemuatlut mentioned this pull request Dec 14, 2017

[FR] Adding UART configurator for TMC2208 drivers. #6044

Closed

thinkyhead merged commit 0ac0324 into MarlinFirmware:bugfix-1.1.x Dec 15, 2017

teemuatlut deleted the TMC_update_PR branch December 16, 2017 21:14

thinkyhead reviewed Mar 18, 2018

View reviewed changes

Marlin/Marlin_main.cpp

#if ENABLED(HAVE_TMC2208)

delay(100);

tmc2208_init();

Copy link

Member

thinkyhead Mar 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@teemuatlut — Re-init not needed in M80?

damicreabox pushed a commit to damicreabox/Marlin that referenced this pull request Sep 14, 2018

[1.1.x] TMC driver update (MarlinFirmware#8712)

6833e45

Uh oh!

[1.1.x] TMC driver update #8712

[1.1.x] TMC driver update #8712

Uh oh!

Conversation

teemuatlut commented Dec 8, 2017

Uh oh!

thinkyhead commented Dec 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

teemuatlut commented Dec 8, 2017

Uh oh!

thinkyhead commented Dec 9, 2017

Uh oh!

teemuatlut commented Dec 9, 2017

Uh oh!

thinkyhead commented Dec 11, 2017

Uh oh!

thinkyhead commented Dec 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

teemuatlut commented Dec 11, 2017

Uh oh!

fiveangle commented Dec 12, 2017

Uh oh!

thinkyhead commented Dec 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

teemuatlut commented Dec 13, 2017

Uh oh!

p3p commented Dec 13, 2017

Uh oh!

teemuatlut commented Dec 13, 2017

Uh oh!

thinkyhead commented Dec 15, 2017

Uh oh!

byebyedizzy commented Dec 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

teemuatlut commented Dec 16, 2017

Uh oh!

teemuatlut commented Dec 16, 2017

Uh oh!

byebyedizzy commented Dec 17, 2017

Uh oh!

thinkyhead commented Dec 25, 2017

Uh oh!

teemuatlut commented Dec 25, 2017

Uh oh!

thinkyhead commented Dec 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmdearras commented Jan 19, 2018

Uh oh!

teemuatlut commented Jan 19, 2018

Uh oh!

jmdearras commented Jan 19, 2018

Uh oh!

bleem313 commented Jan 20, 2018

Uh oh!

teemuatlut commented Jan 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thinkyhead commented Jan 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thinkyhead commented Jan 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bleem313 commented Jan 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

teemuatlut commented Jan 22, 2018

Uh oh!

thinkyhead commented Jan 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

teemuatlut commented Jan 22, 2018

Uh oh!

thinkyhead commented Jan 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

thinkyhead commented Dec 8, 2017 •

edited

Loading

thinkyhead commented Dec 11, 2017 •

edited

Loading

thinkyhead commented Dec 13, 2017 •

edited

Loading

byebyedizzy commented Dec 16, 2017 •

edited

Loading

thinkyhead commented Dec 25, 2017 •

edited

Loading

teemuatlut commented Jan 20, 2018 •

edited

Loading

thinkyhead commented Jan 22, 2018 •

edited

Loading

thinkyhead commented Jan 22, 2018 •

edited

Loading

bleem313 commented Jan 22, 2018 •

edited

Loading

thinkyhead commented Jan 22, 2018 •

edited

Loading

thinkyhead commented Jan 22, 2018 •

edited

Loading