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.15k stars 19.21k forks source link

[2.0.x] [FR] Implement SPI in a Library format for LPC HAL #8334

Closed psavva closed 6 years ago

psavva commented 6 years ago

Dear Marlin Team,

I would like to request support for the TMC2130 drivers, which is implemented by @teemuatlut on his Marlin branch.

To do so, we need to have a SPI Library for the LPC MCU.

The include should work as per the other SPI Libraries, and must follow the same interface as the SPI libraries for AVR/Due/Teensy.

Once done, we would hopefully wait for a pull request from @teemuatlut on the TMC implementation using the SPI Interface

psavva commented 6 years ago

@vilsed As far as I can tell we will always use Software SPI for the LPC and MKS_SBASE, no matter what... ie: pins.h -> HAL_spi_pins.h -> HAL_LPC1768/spi_pins.h -> Which contains an if statement, that will always define LPC_SOFTWARE_SPI for the LPC chipset... Hence that it's not possible to initialize the Hardware SPI for the LCD, and Software SPI on different ports to use with the Stepper Drivers + Others.

I guess something is wrong with the Hardware SPI on the LPC, which is maybe why we always override it with a software SPI?

I think you are right though, We should be able to modify the PIN header on the LCD Adaptor, as to make those pins available for the next SPI modules, such as these TMC Drivers...

I'm really not sure as to why the software spi is always used in this way.. Maybe @Bob-the-Kuhn or @thinkyhead can advise?

p3p commented 6 years ago

The shared SPI on the Re-ARMs controller extension ports would be fine if the RRD Graphical LCD actually followed the SPI specification (it receives data even when not enabled), as it is we do our best to work around the problems (display corruption when accessing sd card) in software but it''s impossible to mitigate entirely. The pins are set up to make sure things mostly work out of the box as it would take a ribbon cable modification to move the lcd to other pins, (with a 20x4 display this is not optional as the extension ports do not have the appropriate pins connected, we had to move those signals to Ethernet pins). Panucatt decided it was more important to reserve pins for Ethernet than to be fully RAMPS compatible more than likely because the design was intended for smoothieware.

Bob-the-Kuhn commented 6 years ago

From the pins_RAMPS_RE_ARM.h file (the RAMPs pin number is a comment)

  //#define MISO_PIN            P0_17  // (50)  system defined J3-10 & AUX-3
  //#define MOSI_PIN            P0_18  // (51)  system defined J3-10 & AUX-3
  //#define SCK_PIN             P0_15  // (52)  system defined J3-9 & AUX-3
  //#define SS_PIN              P1_23  // (53)  system defined J3-5 & AUX-3 - sometimes called SDSS

SPI is a serial interface.

The hardware SPI on the RE-ARM board is normally disabled because the same SPI pins run both the LCD and the SD card on the RepRap Full Graphics board when using the standard LCD adapter. Basically two SPI interfaces are needed. Using only software SPIs means they can use the same pins. The LPC1768 hardware SPI simply doesn't share pins.

psavva commented 6 years ago

@Bob-the-Kuhn the pins will finally be defined in https://github.com/MarlinFirmware/Marlin/blob/bugfix-2.0.x/Marlin/src/HAL/HAL_LPC1768/spi_pins.h

If the LCD doesn't respect the fact that if the SS is disabled, it should ignore data, how does it work today on the LPC chipset? I understand that the software SPI will allow us to use multiple SPI devices without the limitations of the LPC Hardware SPI, however, will the LCD still not be corrupted when accessing the SPI on another device?

Should we not have 2 separate SPI interfaces, using different pins? Ie: one for the LCD, and one for everything else?

vilsed commented 6 years ago

I've connected the drivers to the SPI header on the Re-ARM itself. Communication seems to be working (crap on the LCD), although M122 is pushing "X/Y/Z/E current decreased to [lower and lower value]". Before I had "driver error" message on LCD and printer shutdowns immediately after boot. Had to disable #define STOP_ON_ERROR

EDIT: Reported current is in tens of amps, witch could not be right. Also it threw out a couple of over temperature warnings, although drivers are stone cold

EDIT: It appears that just after boot the current is reduced down to 0 and after that it rolls over to 65486 and goes down from there. Motors aren't running. Just humming and spazing periodically. Disabling LDC/SD support and removing LCD adapter did nothing. M122, M906, M911 aren't working.

Bob-the-Kuhn commented 6 years ago

Two separate SPI interfaces is how it's done on a mega2560 board.

The LPC1768 has fewer pins than the 2560. One of the Re-ARM compromises was to combine the SPI pins when using the standard LCD adapter board.

Two separate SPIs could be used on the Re-ARM system but that would require some type of custom cabling or a new LCD adapter board.

I'd dearly love to use the hardware SPI. The RepRap Full Graphics display is finicky about it's baud rate and duty cycle. The hardware SPI would make that so much easier than with a software SPI.

psavva commented 6 years ago

@Bob-the-Kuhn Could we not use the standard cable as it is now, and utilise the hardware SPI for the LCD alone? Or do you mean to say that the hardware SPI cannot be used due to the SD card using the same pins on the RRD Full graphics smart controller, and the LPC SPI simply doesn't support it?

@vilsed could you send us a photo of your cabling ?

Bob-the-Kuhn commented 6 years ago

On the LPC1768 we MINIMIZE the RRDFG's LCD garbage when in the PRINT SD menu by: 1) using the SDCARD_SORT_ALPHA option which keeps a copy of the directory in RAM 2) going to the RAM for directory info when possible 3) redrawing the display immediately after an SD card access.

Sometimes you'll see garbage on power up when the SD card is being accessed.

vilsed commented 6 years ago

@psavva Sure. It's nearly impossible to understand a thing in this mess though. Basically SDO (blue wire), SDI (yellow wire) and SCK (red wire) pins are connected in parallel for all drivers and go to P0.18, P0.17 and P0.15 pins respectively. CS pins go with individual wires to P1.0, P1.1, P1.8 and P1.4, as per pins.h file.

https://photos.app.goo.gl/fGIk2P69jzRm70dx2

p3p commented 6 years ago

@psavva The issue is that there is no centralised interface to SPI, originally I had both the sdcard and lcd running on hardware SPI but it required modification to all areas of marlin that used SPI to redirect through an API in the HAL, this is how it should be done but it isn't easy to implement and maintain the 8bit AVR performance also any library that uses SPI would ignore our interface, so the current design is a compromise that means we can't use the hardware. I really want to find the time to implement the HALs multi-channel SPI interface but its not happening anytime soon I don't think.

Bob-the-Kuhn commented 6 years ago

The RRDFG LCD section ALWAYS responds to the SCK & MOSI pins. That's just the way it is.

If there were an attractive alternative to the standard LCD adapter then switching to the hardware SPI would be the way to go.

As long as the application only uses SPIs for the LCD's display and the LCD's SD card then the current setup is OK. Definitely not elegant.

FYI - the LPC1768's second hardware SPI is dedicated to the Re-ARM's on-board SD card.

tcm0116 commented 6 years ago

I've seen people add a buffer between the control board and RRDFGD that will disconnect the SPI pins to the LCD when the CS is disabled. That could be an option for someone wanting to share the hardware SPI between the LCD and other devices, such as the TMC2130 drivers, but it would require some custom cabling and additional components.

Bob-the-Kuhn commented 6 years ago

@p3p - Chris, in your vision. does the SPI API actually control the select line or does the application?

In some of the U8G LCD drivers they have a start transmission & end transmission message they send to the com driver and the com driver controls the select. I like that system - especially if we ever have visions of an RTOS.

p3p commented 6 years ago

@Bob-the-Kuhn I made it optional as some devices have really odd timing requirements for the cs line, if you manual enabled the cs it had to be manually disabled, otherwise it was done automatically (also with block transmissions it makes sense, with byte not so much), so it acted like a start/end transmission really.

psavva commented 6 years ago

So it means that we will have the exact same problem when transmitting data the TMC drivers. Ie, we'll have garbage on the LCD. There is really no elegant solution other than a custom cable to utilise a second software SPI for the SD card and other peripherals. It however does mean that we would need to be able to define a second set of software SPI pins. It would also mean that each SPI device should be configurable to which SPI to use... Ie: LCD on SPI1, SD Card and TMC on SPI2

vilsed commented 6 years ago

@psavva Yes, that is exactly what is happening. Although all what that garbage is, is two lines on the LCD. Same as it is while reading from an SD card. The whole screen fills with garbage only once, while booting up.

All in all my situation seems very bizarre to me. It seems that the LPC is talking to TMCs (garbage lines on the screen and "current decreased" messages being pumped over COM), but not really. Tomorrow will try to test the setup with an ATmega, I'm starting to suspect bad drivers, although they did work in simple step/dir mode.

As for the overcrowding on SPI bus, I would be willing to give up LCD and SD support, if that meant that TMC drivers worked. At least for now.

tig33r commented 6 years ago

I gave up with tmc controlled by Marlin. Right now I am using arduino nano to control tmc and it working well. In addition I can use it with smoothieware too.

vilsed commented 6 years ago

It's a shame, really. Re-ARM was supposed to be a plug and play replacement for 2560. Guess nothing is ever simple...

EDIT. I might have found my problem. My drivers are configured to work in standalone mode (SPI jumper closed https://github.com/watterott/SilentStepStick/blob/master/hardware/SilentStepStick-TMC2130_v10.pdf), i.e. SPI pins work like simple configuration pins. It's always simple things like that

psavva commented 6 years ago

Here is an interesting read. https://www.dorkbotpdx.org/blog/paul/better_spi_bus_design_in_3_steps

If any of the electronic engineers know how we can practically solve this issue with a simple fix using a tri-state buffer, please shout out.

I think that it's probably the best solution if there is some small fix on the RRD board itself by implementing the solution as per the article.

vilsed commented 6 years ago

Played around with 2130's a bit. With ATmega they work just fine. M122 gives me all the relevant infos, I can change the drive current and all that. So we can rule out bad drivers/bad wiring harness.

Once more, to summarize my current setup: Re-ARM, RAMPS 1.4, no SD, no LCD (both physically and disabled in marlin), SDO, SDI and SCK lines to P0.18, P0.17 and P0.15 respectively, X_CS, Y_CS, Z_CS and E0_CS to P1.0, P1.1, P1.8 and P1.4. I noticed that these CS pins were being used for ethernet too, although disabling defines for ENET changed nothing.

Anyhow, Happy new year! :)

psavva commented 6 years ago

I tried your exact setup, and cannot get my board to boot once HAVE_TMC2130 is defined.

If I undefined it, all is good and the board boots, of course without the stepper driver...

Once I enable the 2130 define, the LED1 on the Ramps is solid. Not flashing. I cannot access the serial console either...

May I please ask you to zip up your changes to @teemuatlut branch fir the LPC chipset?

psavva commented 6 years ago

After going a little back in history on the comments, I noticed that @vilsed had disabled STOP_ON_ERROR. After disabling this, I have also managed to avoid the board resetting, but of course since there is some error, there is absolutely no feedback from M122 which is supposed to give me some debug information.

I think it's either some bug with communicating with the SPI, or similar...

My setup is exactly the same as that of @vilsed

Re-ARM, RAMPS 1.4, no SD, no LCD (both physically and disabled in marlin), SDOP0.18 SDIP0.17 SCK P0.15 X_CS -> P1.0 I only tested this with 1 driver.

@teemuatlut do you think you'd be able to give advise on how to debug, and further understand what may need to be changed to get the TMC2130 working?

Due to the fact that the halt happens even before I'm able to connect via the serial console, I cannot simply see the error.

psavva commented 6 years ago

I disabled the KILL in tmc_util and re-enabled the STOP_ON_KILL in efforts to get the logs :)

@teemuatlut, Hope this helps to figure out what's going on here....

This is the report from the Serial Monitor: 19:22:03.590 : Printer reset detected - initalizing 19:22:03.590 : -start 8 19:22:03.591 : Stallguard thrs 0 19:22:03.591 : DRVSTATUS X 19:22:03.591 : stallguard X 19:22:03.591 : sg_result 1023 19:22:03.591 : fsactive X 19:22:03.591 : stst X 19:22:03.591 : olb X 19:22:03.591 : ola X 19:22:03.591 : s2gb X 19:22:03.591 : s2ga X 19:22:03.591 : otpw X 19:22:03.591 : ot X 19:22:03.591 : Driver registers: 19:22:03.591 : X = 0x1515:1515:1515:1515 19:22:03.591 : X current decreased to 600 19:22:03.597 : X driver error detected: 19:22:03.599 : overtemperature 19:22:03.601 : short to ground (coil A) 19:22:03.601 : short to ground (coil B) 19:22:03.601 : X 19:22:03.603 : Enabled false 19:22:03.605 : Set current 600 19:22:03.605 : RMS current 581 19:22:03.607 : MAX current 819.210000 19:22:03.609 : Run current 18/31 19:22:03.611 : Hold current 9/31 19:22:03.611 : CS actual 31/31 19:22:03.614 : PWM scale 255 19:22:03.614 : vsense 1=.18 19:22:03.616 : stealthChop true 19:22:03.618 : msteps 0 19:22:03.618 : tstep 4294967295 19:22:03.620 : pwm 19:22:03.620 : threshold 98 19:22:03.622 : [mm/s] 0.000000 19:22:03.622 : OT prewarn true 19:22:03.624 : OT prewarn has 19:22:03.624 : been triggered true 19:22:03.624 : off time 15 19:22:03.626 : blank time 54 19:22:03.628 : hysterisis 19:22:03.628 : -end 12 19:22:03.628 : -start 8 19:22:03.631 : Stallguard thrs 0 19:22:03.631 : DRVSTATUS X 19:22:03.633 : stallguard X 19:22:03.633 : sg_result 1023 19:22:03.636 : fsactive X 19:22:03.636 : stst X 19:22:03.636 : olb X 19:22:03.638 : ola X 19:22:03.638 : s2gb X 19:22:03.638 : s2ga X 19:22:03.642 : otpw X 19:22:03.642 : ot X 19:22:03.644 : Driver registers: 19:22:03.646 : X = 0x1515:1515:1515:1515 19:22:03.649 : X current decreased to 550 19:22:03.729 : N1 M11034 19:22:03.729 : N2 M11536 19:22:03.729 : N4 M11435 19:22:03.731 : N5 M111 S698 19:22:03.734 : N6 T060 19:22:03.734 : N7 M2022 19:22:03.735 : ok 19:22:03.737 : N8 M8019 19:22:03.753 : FIRMWARE_NAME:Marlin bugfix-2.0.x (Github) SOURCE_CODE_URL:https://github.com/MarlinFi_XON_XOFF0 19:22:03.754 : N10 M220 S10080 19:22:03.755 : Cap:EEPROM0 19:22:03.755 : Cap:VOLUMETRIC1 19:22:03.759 : Cap:AUTOREPORT_TEMP1 19:22:03.761 : Cap:PROGRESS0 19:22:03.761 : Cap:PRINT_JOB1 19:22:03.763 : Cap:AUTOLEVEL0 19:22:03.763 : Cap:Z_PROBE0 19:22:03.765 : Cap:LEVELING_DATA0 19:22:03.767 : Cap:BUILD_PERCENT0 19:22:03.769 : Cap:SOFTWARE_POWER0 19:22:03.769 : Cap:TOGGLE_LIGHTS0 19:22:03.771 : Cap:CASE_LIGHT_BRIGHTNESS0 19:22:03.774 : Cap:EMERGENCY_PARSER0 19:22:03.774 : ok 19:22:03.774 : N11 M221 S10080 19:22:03.780 : N12 M111 S684 19:22:03.783 : X:0.000000 Y:0.000000 Z:0.000000 E:0.000000 Count X:0 Y:0 Z:0 19:22:03.783 : ok 19:22:03.783 : N13 T0*8 19:22:03.785 : echo:DEBUG:INFO,ERRORS 19:22:03.785 : ok 19:22:03.787 : echo:Active Extruder: 0 19:22:03.787 : ok 19:22:03.787 : ok 19:22:03.787 : ok 19:22:03.794 : ok 19:22:03.794 : ok 19:22:03.794 : echo:DEBUG:INFO,ERRORS 19:22:03.794 : ok 19:22:03.795 : echo:Active Extruder: 0 19:22:03.795 : ok 19:22:04.095 : X driver error detected: 19:22:04.097 : overtemperature 19:22:04.099 : short to ground (coil A) 19:22:04.099 : short to ground (coil B) 19:22:04.099 : X 19:22:04.099 : Enabled false 19:22:04.101 : Set current 550 19:22:04.103 : RMS current 520 19:22:04.106 : MAX current 733.200000 19:22:04.106 : Run current 16/31 19:22:04.108 : Hold current 8/31 19:22:04.111 : CS actual 31/31 19:22:04.111 : PWM scale 255 19:22:04.114 : vsense 1=.18 19:22:04.114 : stealthChop true 19:22:04.115 : msteps 0 19:22:04.115 : tstep 4294967295 19:22:04.117 : pwm 19:22:04.117 : threshold 98 19:22:04.120 : [mm/s] 0.000000 19:22:04.120 : OT prewarn true 19:22:04.122 : OT prewarn has 19:22:04.126 : been triggered true 19:22:04.126 : off time 15 19:22:04.128 : blank time 54 19:22:04.128 : hysterisis 19:22:04.130 : -end 12 19:22:04.130 : -start 8 19:22:04.132 : Stallguard thrs 0 19:22:04.132 : DRVSTATUS X 19:22:04.135 : stallguard X 19:22:04.137 : sg_result 1023 19:22:04.137 : fsactive X 19:22:04.137 : stst X 19:22:04.139 : olb X 19:22:04.139 : ola X 19:22:04.139 : s2gb X 19:22:04.139 : s2ga X 19:22:04.141 : otpw X 19:22:04.141 : ot X 19:22:04.143 : Driver registers: 19:22:04.145 : X = 0x1515:1515:1515:1515 19:22:04.148 : X current decreased to 500 19:22:04.594 : X driver error detected: 19:22:04.596 : overtemperature 19:22:04.599 : short to ground (coil A) 19:22:04.599 : short to ground (coil B) 19:22:04.599 : X 19:22:04.601 : Enabled false 19:22:04.601 : Set current 500 19:22:04.604 : RMS current 489 19:22:04.606 : MAX current 689.490000 19:22:04.608 : Run current 15/31 19:22:04.608 : Hold current 7/31 19:22:04.610 : CS actual 31/31 19:22:04.612 : PWM scale 255 19:22:04.612 : vsense 1=.18 19:22:04.615 : stealthChop true 19:22:04.615 : msteps 0 19:22:04.617 : tstep 4294967295 19:22:04.617 : pwm 19:22:04.617 : threshold 98 19:22:04.622 : [mm/s] 0.000000 19:22:04.624 : OT prewarn true 19:22:04.626 : OT prewarn has 19:22:04.626 : been triggered true 19:22:04.626 : off time 15 19:22:04.628 : blank time 54 19:22:04.631 : hysterisis 19:22:04.631 : -end 12 19:22:04.631 : -start 8 19:22:04.633 : Stallguard thrs 0 19:22:04.633 : DRVSTATUS X 19:22:04.635 : stallguard X 19:22:04.635 : sg_result 1023 19:22:04.637 : fsactive X 19:22:04.637 : stst X 19:22:04.638 : olb X 19:22:04.640 : ola X 19:22:04.640 : s2gb X 19:22:04.640 : s2ga X 19:22:04.642 : otpw X 19:22:04.642 : ot X 19:22:04.644 : Driver registers: 19:22:04.646 : X = 0x1515:1515:1515:1515 19:22:04.648 : X current decreased to 450 19:22:05.095 : X driver error detected: 19:22:05.097 : overtemperature 19:22:05.099 : short to ground (coil A) 19:22:05.100 : short to ground (coil B) 19:22:05.100 : X 19:22:05.100 : Enabled false 19:22:05.102 : Set current 450 19:22:05.104 : RMS current 428 19:22:05.106 : MAX current 603.480000 19:22:05.106 : Run current 13/31 19:22:05.108 : Hold current 6/31 19:22:05.111 : CS actual 31/31 19:22:05.111 : PWM scale 255 19:22:05.113 : vsense 1=.18 19:22:05.113 : stealthChop true 19:22:05.117 : msteps 0 19:22:05.118 : tstep 4294967295 19:22:05.120 : pwm 19:22:05.120 : threshold 98 19:22:05.122 : [mm/s] 0.000000 19:22:05.122 : OT prewarn true 19:22:05.124 : OT prewarn has 19:22:05.124 : been triggered true 19:22:05.124 : off time 15 19:22:05.127 : blank time 54 19:22:05.129 : hysterisis 19:22:05.129 : -end 12 19:22:05.129 : -start 8 19:22:05.129 : Stallguard thrs 0 19:22:05.131 : DRVSTATUS X 19:22:05.131 : stallguard X 19:22:05.133 : sg_result 1023 19:22:05.138 : fsactive X 19:22:05.138 : stst X 19:22:05.138 : olb X 19:22:05.138 : ola X 19:22:05.140 : s2gb X 19:22:05.140 : s2ga X 19:22:05.140 : otpw X 19:22:05.143 : ot X 19:22:05.143 : Driver registers: 19:22:05.145 : X = 0x1515:1515:1515:1515 19:22:05.147 : X current decreased to 400 19:22:05.593 : X driver error detected: 19:22:05.597 : overtemperature 19:22:05.599 : short to ground (coil A) 19:22:05.600 : short to ground (coil B) 19:22:05.600 : X 19:22:05.602 : Enabled false 19:22:05.602 : Set current 400 19:22:05.604 : RMS current 397 19:22:05.606 : MAX current 559.770000 19:22:05.608 : Run current 12/31 19:22:05.608 : Hold current 6/31 19:22:05.610 : CS actual 31/31 19:22:05.612 : PWM scale 255 19:22:05.612 : vsense 1=.18 19:22:05.615 : stealthChop true 19:22:05.615 : msteps 0 19:22:05.620 : tstep 4294967295 19:22:05.620 : pwm 19:22:05.620 : threshold 98 19:22:05.622 : [mm/s] 0.000000 19:22:05.624 : OT prewarn true 19:22:05.626 : OT prewarn has 19:22:05.626 : been triggered true 19:22:05.628 : off time 15 19:22:05.628 : blank time 54 19:22:05.631 : hysterisis 19:22:05.631 : -end 12 19:22:05.631 : -start 8 19:22:05.634 : Stallguard thrs 0 19:22:05.634 : DRVSTATUS X 19:22:05.636 : stallguard X 19:22:05.636 : sg_result 1023 19:22:05.638 : fsactive X 19:22:05.638 : stst X 19:22:05.638 : olb X 19:22:05.640 : ola X 19:22:05.640 : s2gb X 19:22:05.640 : s2ga X 19:22:05.642 : otpw X 19:22:05.642 : ot X 19:22:05.644 : Driver registers: 19:22:05.646 : X = 0x1515:1515:1515:1515 19:22:05.649 : X current decreased to 350 19:22:06.095 : X driver error detected: 19:22:06.097 : overtemperature 19:22:06.100 : short to ground (coil A) 19:22:06.100 : short to ground (coil B) 19:22:06.100 : X 19:22:06.100 : Enabled false 19:22:06.102 : Set current 350 19:22:06.104 : RMS current 336 19:22:06.106 : MAX current 473.760000 19:22:06.106 : Run current 10/31 19:22:06.108 : Hold current 5/31 19:22:06.110 : CS actual 31/31 19:22:06.110 : PWM scale 255 19:22:06.113 : vsense 1=.18 19:22:06.115 : stealthChop true 19:22:06.115 : msteps 0 19:22:06.117 : tstep 4294967295 19:22:06.117 : pwm 19:22:06.117 : threshold 98 19:22:06.119 : [mm/s] 0.000000 19:22:06.121 : OT prewarn true 19:22:06.123 : OT prewarn has 19:22:06.123 : been triggered true 19:22:06.125 : off time 15 19:22:06.125 : blank time 54 19:22:06.127 : hysterisis 19:22:06.127 : -end 12 19:22:06.130 : -start 8 19:22:06.130 : Stallguard thrs 0 19:22:06.132 : DRVSTATUS X 19:22:06.132 : stallguard X 19:22:06.134 : sg_result 1023 19:22:06.138 : fsactive X 19:22:06.138 : stst X 19:22:06.138 : olb X 19:22:06.138 : ola X 19:22:06.140 : s2gb X 19:22:06.140 : s2ga X 19:22:06.140 : otpw X 19:22:06.142 : ot X 19:22:06.142 : Driver registers: 19:22:06.144 : X = 0x1515:1515:1515:1515 19:22:06.149 : X current decreased to 300 19:22:06.596 : X driver error detected:

teemuatlut commented 6 years ago

All the TMC gcodes (M122, M906, M912, etc) are broken atm for 2.0.x branch. There was a compiler condition oversight (typo) when I ported the last TMC update to the new hierarchy layout from 1.1.x. I made a fix last week but because of new years and me being out of town, I haven't yet made a PR.

Also that's not a valid hex value! Not sure what's going on there.

vilsed commented 6 years ago

@teemuatlut Tried compiling your last week fixed branch, got my ubiquitous COOLCONF compile error. Ported the changes you made to the last branch on my hard drive that compiled successfully. Now TMC Mcodes do work, but somehow drivers don't want to enable. Tried homing, moving axis by Gcode, M17, M18, M84 with no luck. Setting drivers current with M906 seems to work, at least I see changes in M122, so the drivers should be good. Also as per M122 set microsteps are wrong (256 instead of 16). Posting M122 report, maybe you'll be able to see something that I don't.

@psavva Have you had such a condition?

21:14:21.271 : X Y Z E0 21:14:21.275 : Enabled false false false false 21:14:21.277 : Set current 600 600 600 800 21:14:21.280 : RMS current 1049 1049 1049 1436 21:14:21.285 : MAX current 1479.090000 1479.090000 1479.090000 2024.760000 21:14:21.288 : Run current 18/31 18/31 18/31 25/31 21:14:21.292 : Hold current 9/31 9/31 9/31 12/31 21:14:21.295 : CS actual 15/31 15/31 15/31 15/31 21:14:21.297 : PWM scale 0 0 0 0 21:14:21.300 : vsense 0=.325 0=.325 0=.325 0=.325 21:14:21.303 : stealthChop false false false false 21:14:21.305 : msteps 256 256 256 256 21:14:21.309 : tstep 1179648 1179648 1179648 1179648 21:14:21.310 : pwm 21:14:21.311 : threshold 79 79 79 53 21:14:21.316 : [mm/s] 1601.265869 1601.265869 1601.265869 825.879761 21:14:21.319 : OT prewarn false false false false 21:14:21.320 : OT prewarn has 21:14:21.322 : been triggered false false false false 21:14:21.324 : off time 0 0 0 0 21:14:21.326 : blank time 16 16 16 16 21:14:21.327 : hysterisis 21:14:21.329 : -end -3 -3 -3 -3 21:14:21.331 : -start 1 1 1 1 21:14:21.333 : Stallguard thrs 0 0 0 0 21:14:21.335 : DRVSTATUS X Y Z E0 21:14:21.336 : stallguard 21:14:21.338 : sg_result 0 0 0 0 21:14:21.339 : fsactive 21:14:21.340 : stst 21:14:21.341 : olb 21:14:21.341 : ola 21:14:21.342 : s2gb 21:14:21.343 : s2ga 21:14:21.344 : otpw 21:14:21.345 : ot 21:14:21.346 : Driver registers: 21:14:21.348 : X = 0x00:615:00:00 21:14:21.350 : Y = 0x00:615:00:00 21:14:21.351 : Z = 0x00:615:00:00 21:14:21.353 : E0 = 0x00:615:00:00

teemuatlut commented 6 years ago

I can start working on the LPC and DUE platforms for TMC when I find the time. For now the output look really off. The driver registers are reporting non valid HEX values and max current is a floating value. This is not correct.

Some of the values are reported correctly because they are read from the shadow registers held by the library. No communication to the driver happens then.

What is the COOLCONF error?

vilsed commented 6 years ago

It's alright, we all spend as much time as we can spare for this :)

Building your current branch gives me this:

`.piolibdeps\TMC2130Stepper\src\source\TMC2130Stepper_COOLCONF.cpp:16:6: error: prototype for 'void TMC2130Stepper::sgt(int8_t)' does not mat

ch any in class 'TMC2130Stepper' void TMC2130Stepper::sgt( int8_t B ) { MOD_REG(COOLCONF, SGT); } ^~~~~~ In file included from .piolibdeps\TMC2130Stepper\src\source\TMC2130Stepper_COOLCONF.cpp:1:0: .piolibdeps\TMC2130Stepper\src/TMC2130Stepper.h:167:11: error: candidates are: uint8_t TMC2130Stepper::sgt() uint8_t sgt(); ^~~ .piolibdeps\TMC2130Stepper\src/TMC2130Stepper.h:160:8: error: void TMC2130Stepper::sgt(uint8_t) void sgt( uint8_t B); ^~~ .piolibdeps\TMC2130Stepper\src\source\TMC2130Stepper_COOLCONF.cpp:27:8: error: prototype for 'int8_t TMC2130Stepper::sgt()' does not match a ny in class 'TMC2130Stepper' int8_t TMC2130Stepper::sgt() { ^~~~~~ In file included from .piolibdeps\TMC2130Stepper\src\source\TMC2130Stepper_COOLCONF.cpp:1:0: .piolibdeps\TMC2130Stepper\src/TMC2130Stepper.h:167:11: error: candidates are: uint8_t TMC2130Stepper::sgt() uint8_t sgt(); ^~~ compilation terminated due to -fmax-errors=5. .piolibdeps\TMC2130Stepper\src\source\TMC2130Stepper.cpp: In member function 'void TMC2130Stepper::begin()': .piolibdeps\TMC2130Stepper\src\source\TMC2130Stepper.cpp:37:18: error: 'OUTPUT' was not declared in this scope pinMode(_pinEN, OUTPUT); ^~ .piolibdeps\TMC2130Stepper\src\source\TMC2130Stepper.cpp:37:24: error: 'pinMode' was not declared in this scope pinMode(_pinEN, OUTPUT); ^ .piolibdeps\TMC2130Stepper\src\source\TMC2130Stepper.cpp:41:23: error: 'HIGH' was not declared in this scope digitalWrite(_pinEN, HIGH); //deactivate driver (LOW active) ^~~~ .piolibdeps\TMC2130Stepper\src\source\TMC2130Stepper.cpp:41:27: error: 'digitalWrite' was not declared in this scope digitalWrite(_pinEN, HIGH); //deactivate driver (LOW active) ^ .piolibdeps\TMC2130Stepper\src\source\TMC2130Stepper.cpp:42:24: error: 'LOW' was not declared in this scope digitalWrite(_pinDIR, LOW); //LOW or HIGH ^~~ compilation terminated due to -fmax-errors=5. [.pioenvs\LPC1768\libcac\TMC2130Stepper\source\TMC2130Stepper_COOLCONF.o] Error 1 [.pioenvs\LPC1768\libcac\TMC2130Stepper\source\TMC2130Stepper.o] Error 1`

teemuatlut commented 6 years ago

I think you might be running a modified and out dated version of the library. Remove the TMC2130Stepper folder from .libdeps folder and let PIO download it again.

I had an idea that might be worth exploring but I can't do it myself just yet. We might be running the SPI too fast. By default this is setting 0, quoted at "aroud 5MHz". AVR SPI is ran at 2MHz by the TMC library.

There's also an interface mismatch where spiSend, which is used by my LPC SPI class method transfer, doesn't return anything. My assumption was to use spiSend and then spiRec to get the return value. This is not going to work. I will need to make some things not static or more likely more them over to the LPC SPI class and have the Marlin SPI functions direct to those.

vilsed commented 6 years ago

I did modify the library. Patched it up with Bob-the-Kuhns files from Nov 19. Compiling with stock library gives me this:

`|-- v2.0.2 Compiling .pioenvs\LPC1768\lib602\TMC2130Stepper_ID1493\source\SW_SPI.o Compiling .pioenvs\LPC1768\lib602\TMC2130Stepper_ID1493\source\TMC2130Stepper.o

Compiling .pioenvs\LPC1768\lib602\TMC2130Stepper_ID1493\source\TMC2130Stepper_CHOPCONF.o Compiling .pioenvs\LPC1768\lib602\TMC2130Stepper_ID1493\source\TMC2130Stepper_COOLCONF.o .piolibdeps\TMC2130Stepper_ID1493\src\source\TMC2130Stepper.cpp: In member function 'void TMC2130Stepper::send2130(uint8_t, uint32_t*)': .piolibdeps\TMC2130Stepper_ID1493\src\source\TMC2130Stepper.cpp:9:18: error: 'SPI' was not declared in this scope

define TMC_SPI SPI

^ .piolibdeps\TMC2130Stepper_ID1493\src\source\TMC2130Stepper.cpp:69:2: note: in expansion of macro 'TMC_SPI' TMC_SPI.begin(); ^~~ .piolibdeps\TMC2130Stepper_ID1493\src\source\TMC2130Stepper.cpp:71:52: error: 'MSBFIRST' was not declared in this scope TMC_SPI.beginTransaction(SPISettings(16000000/8, MSBFIRST, SPI_MODE3)); ^~~~ .piolibdeps\TMC2130Stepper_ID1493\src\source\TMC2130Stepper.cpp:71:62: error: 'SPI_MODE3' was not declared in this scope TMC_SPI.beginTransaction(SPISettings(16000000/8, MSBFIRST, SPI_MODE3)); ^~~~~ .piolibdeps\TMC2130Stepper_ID1493\src\source\TMC2130Stepper.cpp:71:71: error: 'SPISettings' was not declared in this scope TMC_SPI.beginTransaction(SPISettings(16000000/8, MSBFIRST, SPI_MODE3)); ^ Compiling .pioenvs\LPC1768\lib602\TMC2130Stepper_ID1493\source\TMC2130Stepper_DRV_STATUS.o *** [.pioenvs\LPC1768\lib602\TMC2130Stepper_ID1493\source\TMC2130Stepper.o] Error 1 [ERROR] Took 41.86 seconds`

teemuatlut commented 6 years ago

https://github.com/teemuatlut/Marlin/tree/LPC_TMC2130

If someone wants to give it a try. I am able to initialize and move a TMC2130 powered stepper with that it. The latest version (2.1.0) of the library from github is required and there is no release tag yet.

I would expect Bob's proper SPI implementation to replace the placeholder here as this is not actually even used when TMC_USE_SW_SPI is enabled.

vilsed commented 6 years ago

Awesome! Will try it tonight

psavva commented 6 years ago

I will also give it a go. Anything specific you want to test or should I just try making sure that it works on general with SPI?

teemuatlut commented 6 years ago

I'd just like to know it works for someone else too and not just me. Start with one stepper and move on from there. There are at least two problems that I'm aware of right now. The TMC specific commands don't work on 2.0.x right now and the LPC core doesn't seem handle printing HEX values.

Bob-the-Kuhn commented 6 years ago

The printf command prints hex on the LPC1768.

Roxy-3D commented 6 years ago

The printf command prints hex on the LPC1768.

But if I remember right... You are only allowed to have one % in the format string, right? So you can't do something like:

  printf("%x:%x \n", addr, data);
teemuatlut commented 6 years ago

The printf command prints hex on the LPC1768.

It needs to work through MYSERIAL.print(val, HEX).

tig33r commented 6 years ago

MKS Sbase here:

Motors are working when I do one change in stepper_indirection.cpp (for software tmc software spi definition in conf adv):

define _TMC2130_DEFINE(ST) TMC2130Stepper stepper##ST(ST##_ENABLE_PIN, ST##_DIR_PIN, ST##_STEP_PIN, ST##_CS_PIN, P2_12, P1_23, P1_22)

As you know or not this board has broken hardware spi and sd card for example is working on software spi on (a custom cable is needed.):

define SCK_PIN P1_22 // J8-2 (moved from EXP2 P0.7)

define MISO_PIN P1_23 // J8-3 (moved from EXP2 P0.8)

define MOSI_PIN P2_12 // J8-4 (moved from EXP2 P0.5)

I did not print anything yet, just moving motors from menu. Definitely need some adjustments for pins. Maybe @Bob-the-Kuhn can do shit with this case cause probably software spi changes was by him. Anyway we are almost done with tmc2130 on LPC176X! ;)

Awesome job guys!

teemuatlut commented 6 years ago

Getting the TMC2130 working on LPC was me and a weekend. If you define the TMC_USE_SW_SPI then it will use an implementation internal to the library and the transfer code was taken from Arduino's wiring_shift.c, modified originally by guys at Ultimachine for their Archim2 board.

You can define the pins at your pins_ file so there's no need to edit stepper_indirection.h. See the Re-ARM file for an example.

tig33r commented 6 years ago

Yeah I know but faster option was to edit stepper_indirection.cpp :)

Ofc working with defined parameters in pins_ file. I will test your solution more and give you more detailed feedback.

Bob-the-Kuhn commented 6 years ago

It needs to work through MYSERIAL.print(val, HEX).

On the LPC1768 I tried to just add HEX versions but I got a "can't overload" error.

I can change the decimal printout to HEX but I can't have both at the same time.

If you can point me to where the MYSERIAL.print(val, HEX) works I can see if I can do something similar

p3p commented 6 years ago

But if I remember right... You are only allowed to have one % in the format string, right? So you can't do...

@Roxy-3D not sure where you have picked that up from, (special cut down version maybe?), format strings don't have that limitation, they can be very complex which is why printf / sprintf take quite so much space to link in.

http://www.cplusplus.com/reference/cstdio/printf/

teemuatlut commented 6 years ago

I know it works on AVR and DUE but perhaps the LPC core can provide some guidance.

p3p commented 6 years ago

@Bob-the-Kuhn looks like I just ignored the number base parameter on the HalSerial class print methods (who needed it anyway :wink: )

https://github.com/MarlinFirmware/Marlin/blob/bugfix-2.0.x/Marlin/src/HAL/HAL_LPC1768/serial.h#L152 would need some logic to change the token character depending on it I guess.

Bob-the-Kuhn commented 6 years ago

I tried the following. Either works as desired by themselves but you can't have both at the same time. I get a "can't overload" error when compiling with both enabled.

void print(char value, int = BYTE)             { printf("%c" , value); }
void print(char value, int = HEX)              { printf("%2X" , value); }

I always made the same changes in serial.h and HardwareSerial.cpp.

Hmmm ... I never touched HardwareSerial.h. Something to try tomorrow.

p3p commented 6 years ago

You can't function overload on a default parameter value, you would need to change them something like:

  void print(int value, int nbase = 10) {
    if(nbase == 16) printf("%2X", value);
    else printf("%d");
  }

then update the ln methods to pass through

  void println(int value, int nbase = 10) {
    print(value, nbase);
    println();
  }
vilsed commented 6 years ago

Busy week this one... The new branch compiled with no problems, although SPI communication seems to be down still. No response from motors, M122, M906 spit nothing back out through serial. Tried with all four drivers first, then with only one, nothing. It took me some time to realize that teemuatlut moved the SW SPI pins, but replugging did not help either. @psavva Have you tried your setup yet? I'm starting to expect bad drivers again... :/

tig33r commented 6 years ago

Same situation. Motors are working but no feedback from serial.

teemuatlut commented 6 years ago

If you compiled without any errors, you're working with an outdated repo. There were some recent changes that fixed the TMC commands on 2.0.x branch and they were just merged. I also rebased my branch today to the latest bugfix-2.0.x and you SHOULD have an error about an undefined MYSERIAL. I'm pushing a fix as soon as I confirm that the steppers are moving and serial commands go through.

Edit: Stepper and commands both work. Please update the fork to the latest commit 516cae631ca4cd4bb30a19b7736ad368b7b48807 (Update to new serial naming scheme)

tig33r commented 6 years ago

Compiled newest version and error at start. Driver error. Printer halted. Please restart.

Connecting... echo:SD init fail Printer is now online. echo:SD init fail X driver error detected: X Y Enabled false false Set current 1200 1200 RMS current 1160 1160 MAX current 1635.600000 1635.600000 Run current 20/31 20/31 Hold current 10/31 10/31 CS actual 0/31 0/31 PWM scale 0 0 vsense 0=.325 0=.325 stealthChop false false msteps 256 256 tstep 0 0 pwm threshold 0 0 [mm/s] - - OT prewarn false false OT prewarn has been triggered false false off time 0 0 blank time 16 16 hysterisis -end -3 -3 -start 1 1 Stallguard thrs 0 0 DRVSTATUS X Y stallguard sg_result 0 0 fsactive stst olb ola s2gb s2ga otpw ot Driver registers: X = 0x 0 0: 0 0: 0 0: 0 0 Y = 0x 0 0: 0 0: 0 0: 0 0 Error:Printer halted. kill() called! [ERROR] Error:Printer halted. kill() called!

teemuatlut commented 6 years ago

Check your wiring. And attach your configs (both + pins) and preferably an image of your wiring setup.