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

Board SKR Pro 1.1 needs a fork of TMCStepper #15530

Closed
wants to merge 4 commits into from
Closed

Board SKR Pro 1.1 needs a fork of TMCStepper #15530

wants to merge 4 commits into from

Conversation

G-Pereira
Copy link
Contributor

Description

This board needs a special fork from bigtreetech themselves in order to work with UART enabled drivers (Software Serial)

Benefits

It doesn't work without this

Related Issues

#14615

@boelle
Copy link
Contributor

boelle commented Oct 12, 2019

i cant see for sure, but is the file placed next to the example configs or in the main marlin folder?

@extesy
Copy link

extesy commented Oct 12, 2019

@G-Pereira It works for me without using bigtreetech's fork. Make sure you are using latest versions of everything by deleting .pio folder.

@viper93458
Copy link

viper93458 commented Oct 13, 2019

I was able to compile without the bigtreetecfh fork yesterday but as I started getting a different compile issue regarding PID_AUTOTUNE and an array bounds issue, I disabled the PID_AUTOTUNE_MENU option and since doing so (and clearing the pio folder), I am back to getting the TMC compile errors regarding software serial.

I have tried doing the following in platformio.ini to force the latest ststm32 code:

[env:BIGTREE_SKR_PRO]
#platform = ststm32
platform = https://github.com/platformio/platform-ststm32.git

but doing so causes this error instead:
image

-William

@G-Pereira
Copy link
Contributor Author

@extesy @boelle To trigger the error you have to have certain configurations enabled that actually need those libraries. Here it is my configuration files for you to recreate my error at your end.

Marlin-confs.zip

@viper93458 I had exactly the same situation. At a certain point, I got it to compile.

@G-Pereira
Copy link
Contributor Author

With teemuatlut's master version

@extesy
Copy link

extesy commented Oct 15, 2019

@G-Pereira But your PR does not affect only those configurations, it affects all TMC drivers on this board, including the ones that already work like 5160s. It forces them to use a barely maintained fork. I don't want to use that fork for my 5160s. Why not address the root problem and make official library compatible instead?

@sjasonsmith
Copy link
Contributor

sjasonsmith commented Oct 15, 2019

@extesy, I still can't build with software serial without jumping through hoops to add other dependencies. If you have it working, can you post your platformio.ini?

The official TMCStepper repo will compile for the STM32F4 now, but it isn't yet released/versioned. You can compile if you combine it's master with the FYSETC SoftwareSerialM library. I haven't actually tested this exact combination, as my board isn't sitting next to me at the moment.

I personally feel better with a potentially stale serial library than definitely stale stepper library.

You will also need to add the -DHAVE_SW_SERIAL build flag.

  TMCStepper=https://github.com/teemuatlut/TMCStepper#master
  SoftwareSerialM=https://github.com/FYSETC/SoftwareSerialM#master

Edit: Upon testing this is causing watchdog resets for me, so it's not actually a good option.

This would of course all be temporary until the newer STM32F4 platform is used (with it's own SoftwareSerial) and TMCStepper updates its released version.

@AbelCS
Copy link

AbelCS commented Oct 15, 2019

@extesy, I still can't build with software serial without jumping through hoops to add other dependencies. If you have it working, can you post your platformio.ini?

The official TMCStepper repo will compile for the STM32F4 now, but it isn't yet released/versioned. You can compile if you combine it's master with the FYSETC SoftwareSerialM library. I haven't actually tested this exact combination, as my board isn't sitting next to me at the moment.

I personally feel better with a potentially stale serial library than definitely stale stepper library.

You will also need to add the -DHAVE_SW_SERIAL build flag.

  TMCStepper=https://github.com/teemuatlut/TMCStepper#master
  SoftwareSerialM=https://github.com/FYSETC/SoftwareSerialM#master

This would of course all be temporary until the newer STM32F4 platform is used (with it's own SoftwareSerial) and TMCStepper updates its released version.

Maybe we can bump this issue to get a temporary fix on TMCStepper library: teemuatlut/TMCStepper#75

@extesy
Copy link

extesy commented Oct 15, 2019

I've posted my configs in #15538 (comment)

@sjasonsmith
Copy link
Contributor

I've posted my configs in #15538 (comment)

OK, as was pointed out in that issue, your configs don't use TMC steppers, so it isn't relevant to this problem. The problems involving TMCStepper only arise when you are have actually enabled a TMC UART with Software Serial.

@sjasonsmith
Copy link
Contributor

I haven't actually tested this exact combination, as my board isn't sitting next to me at the moment.

I just tried my suggested combination and I'm getting watchdog resets, so don't use it unless you are prepared to debug that! It works when I build against the BigTreeTech master.

@extesy
Copy link

extesy commented Oct 15, 2019

@sjasonsmith

as was pointed out in that issue, your configs don't use TMC steppers

What are you talking about? TMC5160 is not a TMC stepper?

@sjasonsmith
Copy link
Contributor

@extesy

What are you talking about? TMC5160 is not a TMC stepper?

My mistake, I looked at the image posted and didn't realize it was a diff. I looked at the left side and saw A4988.

I confirmed I can build yours without modifications. I think the TMCStepper library always uses SPI for the 5160, even though the chip also supports serial. This is why you aren't seeing the same issues other have with 2208 or 2209 drivers.

I verified that if I change the stepper type to 2208, your configs won't build anymore.

@extesy
Copy link

extesy commented Oct 15, 2019

@sjasonsmith This matches my experience as well. Since stm32 now includes software serial support, the proper fix would be to use the latest version of stm32 in Marlin. I know that it currently won't compile, but that is exactly what should be fixed, instead of using BIQU fork.

@LinoBarreca
Copy link
Contributor

@sjasonsmith This matches my experience as well. Since stm32 now includes software serial support, the proper fix would be to use the latest version of stm32 in Marlin.

does it? any links/sources for this?

@G-Pereira
Copy link
Contributor Author

using the fork is the only current working state so, in my opinion, it makes no sense to keep marlin broken for this setup because of that. It would make sense if it was Marlin's fault/bug but it isn't.

@extesy
Copy link

extesy commented Oct 22, 2019

@LinoBarreca stm32duino/Arduino_Core_STM32#645

@LinoBarreca
Copy link
Contributor

using the fork is the only current working state so, in my opinion, it makes no sense to keep marlin broken for this setup because of that. It would make sense if it was Marlin's fault/bug but it isn't.

No, it's not the only way.
See here:
teemuatlut/TMCStepper#75 (comment)
I compiled it without problems.

..and actually it's Marlin's "fault", let me explain why:
TMCStepper needs SoftwareSerial if board is SKR_PRO and a driver isn't defined as standalone (so it's uart)
Now...

STM32 version 1.7.0 already HAS SoftwareSerial built-in.
STM32 1.6 has not.

If you upgrade to
platform = https://github.com/platformio/platform-ststm32.git
(the current dev branch which is updated to stmcore 1.7.0)
marlin can't use the timers anymore because the type stimer_t (in 1.6) has been replaced with HardwareTimer (in 1.7)

so...we have 3 options:

  1. use outdated TMCStepper from BTT
  2. add third party SoftwareSerial (as suggested also in Marlin\src\HAL\HAL_STM32F1\SoftwareSerial.cpp)
  3. modify Marlin in this way:
  • get rid of any marlin-defined dummy SoftwareSerial.cpp for STM32
  • modify timer.h & cpp to use the new HardwareTimer which will be the future from now on.
  • reference the git version of stm32 until they release it.

@extesy
Copy link

extesy commented Oct 23, 2019

@LinoBarreca stm32 platform has been released, platformio sent the announcement email: https://community.platformio.org/t/st-stm32-dev-platform-5-7-0-mbed-os-5-14-arduino-core-1-7-0/10160

@LinoBarreca
Copy link
Contributor

@extesy lol...one hour ago....so fast was kinda unexpected since a few hours ago they didn't mention it.....

@@ -487,7 +487,7 @@ build_flags = ${common.build_flags}
lib_deps =
U8glib-HAL=https://github.com/MarlinFirmware/U8glib-HAL/archive/bugfix.zip
LiquidCrystal@1.3.4
TMCStepper@>=0.5.0,<1.0.0
https://github.com/bigtreetech/TMCStepper#master
Copy link
Contributor

Choose a reason for hiding this comment

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

prefer this format : the team prefer to avoid git requirement....
TMCStepper=https://github.com/bigtreetech/TMCStepper/archive/master.zip

@extesy
Copy link

extesy commented Oct 23, 2019

@LinoBarreca So your option 3 looks like the best and cleanest solution to this problem. Are you familiar enough with Marlin codebase to come up with PR?

@LinoBarreca
Copy link
Contributor

LinoBarreca commented Oct 23, 2019

@extesy familiar enough with marlin to do a PR yes. Familiar enough with the new HardwareTimer class and its low level implementation on STM32 definitely no. My knowledge doesn't go so low-level, unfortunately.

Edit: I found the documentation here:
https://github.com/stm32duino/wiki/wiki/HardwareTimer-library
I can study it and see what I can get. ;)

@LinoBarreca
Copy link
Contributor

@extesy okay. I tried to translate the old logic to the new one. it compiled good. I don't know if it will work though since I don't have a SKR Pro. I'll make a pull request for the others to review it.

@thinkyhead
Copy link
Member

The TMCStepper library has recently been updated to allow for single-wire UART. Please test the latest Marlin with the master branch (or most recent version) of TMCStepper to see that it compiles properly.

@thinkyhead thinkyhead closed this Oct 25, 2019
@AnHardt
Copy link
Contributor

AnHardt commented Oct 26, 2019

The TMCStepper library has recently been updated to allow for single-wire UART. Please test the latest Marlin with the master branch (or most recent version) of TMCStepper to see that it compiles properly.

For the BTT SKR PRO V1.1 this does not make much sense.
image

The second pins are not usable for anything else. They are hardwired together.

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

Successfully merging this pull request may close these issues.

10 participants