search

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.3k stars 19.25k forks source link

[bugfix-2.0.x] error: 'stepperX' was not declared in this scope #11389

Closed ModMike closed 6 years ago

ModMike commented 6 years ago

Description

Compilation error when #define X_DRIVER_TYPE TMC2660 in configuration.h (for all axes)>

Steps to Reproduce

  1. Use Azteeg X5GT or Ender 3 example configuration.h & configuration_adv.h

  2. Starting at line 565 In Configuration.h

define X_DRIVER_TYPE TMC2660

define Y_DRIVER_TYPE TMC2660

define Z_DRIVER_TYPE TMC2660

etc...

  1. In Configuration_adv.h line 1171 comment out #define STEALTHCHOP

  2. Compile LPC1768

Compilation Error:

Marlin/src/gcode/feature/trinamic/M906.cpp: In static member function 'static void GcodeSuite::M906()':
Marlin/src/gcode/feature/trinamic/M906.cpp:37:46: error: 'stepperX' was not declared in this scope
#define TMC_SET_CURRENT(Q) tmc_set_current(stepper##Q, value)
^

Marlin/src/gcode/feature/trinamic/M906.cpp:46:27: note: in expansion of macro 'TMC_SET_CURRENT'
if (index == 0) TMC_SET_CURRENT(X);
^~~~~~~~~~~~~~~
Marlin/src/gcode/feature/trinamic/M906.cpp:37:46: note: suggested alternative: 'strupr'
#define TMC_SET_CURRENT(Q) tmc_set_current(stepper##Q, value)
^
Marlin/src/gcode/feature/trinamic/M906.cpp:46:27: note: in expansion of macro 'TMC_SET_CURRENT'
if (index == 0) TMC_SET_CURRENT(X);
^~~~~~~~~~~~~~~
Marlin/src/gcode/feature/trinamic/M906.cpp:37:46: error: 'stepperY' was not declared in this scope
#define TMC_SET_CURRENT(Q) tmc_set_current(stepper##Q, value)
^
Marlin/src/gcode/feature/trinamic/M906.cpp:54:27: note: in expansion of macro 'TMC_SET_CURRENT'
if (index == 0) TMC_SET_CURRENT(Y);
^~~~~~~~~~~~~~~
Marlin/src/gcode/feature/trinamic/M906.cpp:37:46: note: suggested alternative: 'strupr'
#define TMC_SET_CURRENT(Q) tmc_set_current(stepper##Q, value)
^
Marlin/src/gcode/feature/trinamic/M906.cpp:54:27: note: in expansion of macro 'TMC_SET_CURRENT'
if (index == 0) TMC_SET_CURRENT(Y);
^~~~~~~~~~~~~~~
Marlin/src/gcode/feature/trinamic/M906.cpp:37:46: error: 'stepperZ' was not declared in this scope
#define TMC_SET_CURRENT(Q) tmc_set_current(stepper##Q, value)
^
Marlin/src/gcode/feature/trinamic/M906.cpp:62:27: note: in expansion of macro 'TMC_SET_CURRENT'
if (index == 0) TMC_SET_CURRENT(Z);
^~~~~~~~~~~~~~~

etc...

Archive.zip

GMagician commented 6 years ago

Don't know if TMC2660 has something to do with TMC26X but former seems has no code to handle it

teemuatlut commented 6 years ago

You can try this, but I really don't know what the current condition for TMC2660 is at this point on upstream. I haven't yet merged in my efforts to support it.

ModMike commented 6 years ago

Thank you.

ModMike commented 6 years ago

The only thing I changed was the board type to AZTEEG_X5_GT and drivers to TMC2660 in the configuration.h and I get:

Marlin/src/HAL/HAL_LPC1768/../../inc/SanityCheck.h:1566:4: error: #error "STEALTHCHOP requires TMC2130 or TMC2208 stepper drivers.

By the way, that is the same error I get in the official build. When I try to disable stealthchop I get a cascade of errors. As you know, the TMC2660 does not have stealthchop but it gets recognized as a TMC and it seems to force the enabling of stealthchop.

There is a very good chance I am not properly disabling it. Let me know, thanks!

ModMike commented 6 years ago

Here is the error I get when I disable #DEFINE STEALTHCHOP in configuration_adv.h:

Marlin/src/gcode/feature/trinamic/M906.cpp: In static member function 'static void GcodeSuite::M906()':
Marlin/src/gcode/feature/trinamic/M906.cpp:37:46: error: 'stepperX' was not declared in this scope
#define TMC_SET_CURRENT(Q) tmc_set_current(stepper##Q, value)
^

Marlin/src/gcode/feature/trinamic/M906.cpp:46:27: note: in expansion of macro 'TMC_SET_CURRENT'
if (index == 0) TMC_SET_CURRENT(X);
^~~~~~~~~~~~~~~
Marlin/src/gcode/feature/trinamic/M906.cpp:37:46: note: suggested alternative: 'strupr'
#define TMC_SET_CURRENT(Q) tmc_set_current(stepper##Q, value)
^
Marlin/src/gcode/feature/trinamic/M906.cpp:46:27: note: in expansion of macro 'TMC_SET_CURRENT'
if (index == 0) TMC_SET_CURRENT(X);
^~~~~~~~~~~~~~~

it repeats for Y and Z as well.

teemuatlut commented 6 years ago

I think I'll suggest this branch for you https://github.com/teemuatlut/Marlin/tree/bf2_tmc2660

As said, support from my side is not merged yet so I wouldn't consider Marlin supporting them at this point. I don't know how the previous TMC2660 implementation was done and there are no references to them in stepper_indirection header.

ModMike commented 6 years ago

I appreciate you taking the time to answer me. I am not trying to be difficult, I am just trying to get Azteeg X5 GT fully and officially supported with the TMC2660s. I really like the idea of the bigfoot drivers and the Azteeg product line in general.

It's unfortunate that you did the work and it can't be merged. I respect your time and efforts, let me know if anything changes.

teemuatlut commented 6 years ago

The operative word here is "yet". It's mostly a matter of having a couple bigger changes that are prerequisites before I can make a PR for the TMC2660. One major change is the transition to a whole new TMC library. That needs to be its own addition and I expect there to be a period where we find newly introduced bugs.

That branch I linked has been reported to work with your setup so give it a go and maybe it'll work for you too.

In the end I try to make everything I do part of the upstream branch so other users can benefit as well.

thinkyhead commented 6 years ago

See if the current bugfix branches are better. Evaluation of axis stepper types is now deferred, so should it produce more correct compilation results.

ModMike commented 6 years ago

thinkyhead, thank you for responding.

I just tried with latest branch and same error. It looks like the TMC 2660 is mistakenly identified as stealthchop capable. I attached my configuration.h, only the board and drivers were changed from stock.

Here is the error:

In file included from Marlin/src/HAL/HAL_LPC1768/../../inc/MarlinConfig.h:37:0,
from Marlin/src/HAL/HAL_LPC1768/HAL.cpp:23:

Marlin/src/HAL/HAL_LPC1768/../../inc/SanityCheck.h:1566:4: error: #error "STEALTHCHOP requires TMC2130 or TMC2208 stepper
 drivers."
#error "STEALTHCHOP requires TMC2130 or TMC2208 stepper drivers."
^~~~~
In file included from Marlin/src/HAL/HAL_LPC1768/../../inc/MarlinConfig.h:37:0,
from Marlin/src/HAL/HAL_LPC1768/HAL_spi.cpp:51:
Marlin/src/HAL/HAL_LPC1768/../../inc/SanityCheck.h:1566:4: error: #error "STEALTHCHOP requires TMC2130 or TMC2208 stepper
 drivers."
#error "STEALTHCHOP requires TMC2130 or TMC2208 stepper drivers."
^~~~~
In file included from Marlin/src/HAL/HAL_LPC1768/../../inc/MarlinConfig.h:37:0,
from Marlin/src/HAL/HAL_LPC1768/HAL_timers.cpp:31:
Marlin/src/HAL/HAL_LPC1768/../../inc/SanityCheck.h:1566:4: error: #error "STEALTHCHOP requires TMC2130 or TMC2208 stepper
 drivers."
#error "STEALTHCHOP requires TMC2130 or TMC2208 stepper drivers."
^~~~~
In file included from Marlin/src/HAL/HAL_LPC1768/../../inc/MarlinConfig.h:37:0,
from Marlin/src/HAL/HAL_LPC1768/LPC1768_PWM.cpp:74:
Marlin/src/HAL/HAL_LPC1768/../../inc/SanityCheck.h:1566:4: error: #error "STEALTHCHOP requires TMC2130 or TMC2208 stepper
 drivers."
#error "STEALTHCHOP requires TMC2130 or TMC2208 stepper drivers."
^~~~~
*** [.pioenvs/LPC1768/src/src/HAL/HAL_LPC1768/HAL.o] Error 1
*** [.pioenvs/LPC1768/src/src/HAL/HAL_LPC1768/HAL_spi.o] Error 1
*** [.pioenvs/LPC1768/src/src/HAL/HAL_LPC1768/HAL_timers.o] Error 1
*** [.pioenvs/LPC1768/src/src/HAL/HAL_LPC1768/LPC1768_PWM.o] Error 1

Here is the error with stealthchop commented out:

Compiling .pioenvs/LPC1768/src/src/gcode/gcode.o
Marlin/src/gcode/feature/trinamic/M906.cpp: In static member function 'static void GcodeSuite::M906()':
Marlin/src/gcode/feature/trinamic/M906.cpp:37:46: error: 'stepperX' was not declared in this scope
#define TMC_SET_CURRENT(Q) tmc_set_current(stepper##Q, value)
^
Marlin/src/gcode/feature/trinamic/M906.cpp:46:27: note: in expansion of macro 'TMC_SET_CURRENT'

if (index == 0) TMC_SET_CURRENT(X);
^~~~~~~~~~~~~~~
Compiling .pioenvs/LPC1768/src/src/gcode/geometry/G17-G19.o
Marlin/src/gcode/feature/trinamic/M906.cpp:37:46: note: suggested alternative: 'strupr'
#define TMC_SET_CURRENT(Q) tmc_set_current(stepper##Q, value)
^
Marlin/src/gcode/feature/trinamic/M906.cpp:46:27: note: in expansion of macro 'TMC_SET_CURRENT'
if (index == 0) TMC_SET_CURRENT(X);
^~~~~~~~~~~~~~~

I only included X for brevity's sake. It repeats for Y and Z.

To the non-programmer like myself, it looks like we are a few lines of code away, but I could of course be VERY wrong.

Configuration.h.zip

ModMike commented 6 years ago

By the way Scott, I saw your interview and you were extremely clear and well spoken. It was a very informative piece.

thinkyhead commented 6 years ago

@teemuatlut — I need to sort out some defines. This is what's causing the problem here:

#define HAS_TRINAMIC ( HAS_DRIVER(TMC2130) || HAS_DRIVER(TMC2208) || HAS_DRIVER(TMC2660) )

In Configuration_adv.h the block that contains STEALTHCHOP and other options is wrapped by:

#if HAS_TRINAMIC
  . . .
#endif // HAS_TRINAMIC

Should the options in Configuration_adv.h only apply to the 2130 and 2208? Or, are there some options there that should apply to all three? If these should only apply to 2130 and 2208 then we need to change the condition. In other places the HAS_TRINAMIC condition is used to include tmc_util.h, so it seems that HAS_TRINAMIC should include the 2660 for those uses.

teemuatlut commented 6 years ago

You can remove the TMC2660 from HAS_TRINAMIC and we can then re-add once I merge in my implementation for it.

In the HAS_TRINAMIC configuration block, some of the options apply to all drivers, some of the options only to a subset. Like for example the R_SENSE would apply to all current and future TMC chips but enabling Stealthchop to only those that support it. The options themselves do not need to be enclosed in chip specific conditions, this is why I made the collective stealthchop and stallguard condition "has" macros.

To address the sanitycheck:1566 error. From what I can tell the condition works as intended. With TMC2660 defined for XYZE and with STEALTHCHOP enabled, the sanitycheck is triggered. When the option is turned off the compile continues. TMC2660 is definitely (most probably) not detected as being stealthchop capable.

The reason why there is an error in M906.cpp is because there is no stepperX declared in stepper_indirection.h. This is because as far as I'm concerned upstream Marlin does not support the TMC2660 driver. I left it in the macros because there was some support for it but I don't know how it works/worked. Perhaps it would've been better left out for now and not claim support for the driver.

thinkyhead commented 6 years ago

To address the sanitycheck:1566 error. From what I can tell the condition works as intended. With TMC2660 defined for XYZE and with STEALTHCHOP enabled, the sanitycheck is triggered. When the option is turned off the compile continues.

If TMC2660 is removed from HAS_TRINAMIC that will solve this issue, since none of the Configuration_adv.h extra TMC options will be enabled, so STEALTHCHOP will be disabled too.

thinkyhead commented 6 years ago

Perhaps it would've been better left out for now and not claim support for the driver.

Well, we should include it so we can at least select timer values for it. So, would it be better to just have the 2660 always operate as STANDALONE, with no extra distinction made for the non-standalone option (for now)?

ModMike commented 6 years ago

I was told that by Panucatt that the TMC2660 on the Azteeg X5 GT cannot operate in stand alone mode. I tried but could never get anything to move. I may of course have misunderstood what Roy said.

teemuatlut commented 6 years ago

My take on it would be to remove the config options for it altogether. From what I can tell, the previous existing TMC2660 support for was only for HAL_STM32F7 and likely it was mostly for developing purposes. From what I can remember, it is based on my TMCStepper library and was added to the HAL so that the HAL development was able to continue. The addition was proposed full well knowing that it's a temporary solution and the final goal is that TMC2660 is treated the same was as the rest of the TMC drivers (except for anything TMC26x which I don't have anything to do with). So I don't think there's any reason to just allow the standalone configs either if it was part of just one specific HAL and just for temporary developing purposes.

thinkyhead commented 6 years ago

I started a PR at #11485 to remove the influence of TMC2660. I could remove TMC2660 altogether, but what should someone using that option now replace it with in their configuration?

teemuatlut commented 6 years ago

The way you've done it right now, TMC2660 would still be the correct choice. The difference now is that it only affects the driver timings. I haven't look into it, but I believe the STM32F7 HAL handles initiating the drivers on its own.

github-actions[bot] commented 4 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.