TMCStepper icon indicating copy to clipboard operation
TMCStepper copied to clipboard

Disable softeware serial based on external definition.

Open DickyMcCockpants opened this issue 3 years ago • 3 comments
trafficstars

The current paradigm (in V1 branch) for when to use software serial is a mess in TMC_HAL.h

Eg. LPC1768 checks for MARLIN_FIRMWARE and disables software spi outright. On the other hand Arduino AVR uses SoftwareSpi by default.

Can we have an option to force hardware/software communication for platforms that support both? That way whoever uses the library can choose how it functions. E.g. Someone using this library in marlin on AVR can use either hardware or software serial, and deciding which to use depends heavily on use-case.

So replace the checks for marlin with a check for something like TMC_HARDWARE_SERIAL or TMC_SOFTWARE_SERIAL, which can optionally be defined in the project calling this library. (e.g. in marlin config)

Trying to fit external use cases is a rabbit hole, I think it's better to give whatever is calling the library options to decide how this library will act.

DickyMcCockpants avatar Jan 02 '22 17:01 DickyMcCockpants

LPC1768 checks for MARLIN_FIRMWARE and disables software spi outright

It checks for Marlin because Marlin has its own implementation for SPI that I don't want to bypass. For the LPC it's all Software SPI and the framework doesn't provide an SPI.h header. This is all to give more control to Marlin so it can better sort out multi slave setups. Software SPI is NOT disabled in any way. Quite the opposite.

On the other hand Arduino AVR uses SoftwareSpi by default.

I don't know what you mean by this. There are no defaults.

Can we have an option to force hardware/software communication for platforms that support both?

Use the appropriate constructor.

Someone using this library in marlin on AVR can use either hardware or software serial

This has been very much possible for a long time.

So replace the checks for marlin with a check for something like TMC_HARDWARE_SERIAL or TMC_SOFTWARE_SERIAL, which can optionally be defined in the project calling this library. (e.g. in marlin config)

Marlin configs are not passed on to external libraries, unless they're doing some scripting magic to append compile flags.


You seem to be mixing your focus between uart and spi. Are you talking about one or the other or both at the same time?

teemuatlut avatar Jan 02 '22 18:01 teemuatlut

In the Release V1 branch TMC_HAL.h,

#if defined(ARDUINO_ARCH_AVR)

#include <Arduino.h>
#include <SoftwareSerial.h>
#include <SPI.h>

#define SW_CAPABLE_PLATFORM true

Then in TMCStepper.h

#elif defined(AVR) || defined(TARGET_LPC1768) || defined(ARDUINO_ARCH_STM32) #define SW_CAPABLE_PLATFORM true

followed by

#if SW_CAPABLE_PLATFORM #include <SoftwareSerial.h> #endif

I think the easiest way to resolve is to change the define for SW_CAPABLE_PLATFORM to if not defined. That way the implementing project can define it as false to force the exclusion of the SoftwareSerial library from this library.

I made this modification to a copy of the master branch, and the only issue that appeared was the TXRX_pin not being defined for the TMC2208Stepper.cpp which was resolved by changing the conditionals it was in to check for TXRX_pin as well as ARDUINO_ARCH_AVR. This way it retains all existing functionality.

I'll look through the Release V1 branch today and see if it will work in your newer version.

DickyMcCockpants avatar Jan 02 '22 18:01 DickyMcCockpants

My core issue stems from your library's use of SoftwareSerial initializing interrupts, and I think it might also affect other projects where this library is used and interrupts are also needed.

I hadn't looked at V1 as much as the master branch, and I misread about the LPC1768 where I saw it calling an alternate library for marlin. However the concept is still sound. If the Arduino platform's SW_CAPABLE_PLATFORM define is changed to if not defined it would allow the project calling it to prevent SoftwareSerial from being included without affecting any existing functionality (as long as the changes to TMC2208Stepper.cpp are also made).

DickyMcCockpants avatar Jan 02 '22 18:01 DickyMcCockpants