simon-jouet / ESP32Controller

ESP32-based 3D printer controller
120 stars 29 forks source link

[Question] Where to define I2S_STEPPER_STREAM and any ideas about temperature readings? #5

Closed felixstorm closed 3 years ago

felixstorm commented 5 years ago

Hi Simon,

thanks for your great work! I upgraded my Creality Ender 3 to use an ESP32 with TMC2208 stepper drivers connected to I2S based on your coding and on your schematic here in the r2 branch and it works like a charm - just finished my first print today!

Two questions came up that you might have an answer to:

Thanks again for your great work and kind regards, Felix

simon-jouet commented 5 years ago

Hi @felixstorm,

Great to hear that you are using the HAL especially with the I2S not many people do unfortunately (although I guess when a board becomes available more people will)

Where did you define I2S_STEPPER_STREAM? I thought it would be best suited in pins_ESP32.h but then it seems to be missing inside stepper.cpp (all moves ended up terribly slow as STEPPER_TIMER_RATE had been defined as if used without I2S). I ended up putting it in platformio.ini as a build flag, but maybe you have a better idea?

I will have to double check with my local version when I have access to it but IIRC it was defined in the HAL.h of the ESP32's hal.

Temperature readings vary massively. This might have to do with the ESP32s ADC - is that just the way it is or have you been able to figure out a better solution?

So the ESP32 ADC is far from perfect but it should be more stable than what you describe (I also think that Marlin can be tweaked to work better especially when the ADC can be sampled quickly). The first thing to check is which version of the ESP32 you are using, the ESP R1 is much better because it contains a factory built-in calibration curve for the ADC as well the proper VREF. If you use a R0 then you can calibrate the VREF by hand (although I would just recommend switching to an R1). You can also check the voltage reference of your voltage divider make sure it's very close to 3.3V. Check that the resistors you have used have a good tolerance. And finally make sure to retune your PID.

Hope that helps, I haven't really touched this in a while, but I'm still planning to get back to it at some point :)

Simon

felixstorm commented 5 years ago

@simon-jouet - thanks for your reply. The I2S path really seems a very clever (and cheap) way to go to get more GPOs and perfect stepper timing. I really do not know either why people do not use it more often.
For me it was quite simple: I wanted to upgrade my Ender-3 to TMC2208 and WiFi, but also wanted to keep LCD, SD card etc., so I needed the pins and so I thought to give it a try.

About I2S_STEPPER_STREAM: I already did some tweaks to my local copy to be able to define I2S_STEPPER_STREAM in pins*.h (where it makes sense the most IMHO since the pin numbers defined there target I2S 128+), I just need to verify that it does actually work, rebase against a current version of bugfix-2.0.x and then I would file a pull request to https://github.com/MarlinFirmware/Marlin.

I will post it here after I did the polishing so you can have a look and potentially object.

felixstorm commented 5 years ago

About the temperature readings: I did do a PID retune, but I do have to check the ESP revision. It's not that the readings are off a certain amount or percentage, but they go up and down permanently between 5 and 8 °C. Do you remember if you had stable temperature readings on your printer?

felixstorm commented 5 years ago

Ok, this would be the patch for I2S_STEPPER_STREAM:

 Marlin/src/HAL/HAL_ESP32/HAL_timers_ESP32.h | 6 ++++++
 Marlin/src/pins/pins_ESP32.h                | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/Marlin/src/HAL/HAL_ESP32/HAL_timers_ESP32.h b/Marlin/src/HAL/HAL_ESP32/HAL_timers_ESP32.h
index 7fbaa5222..3d2d777de 100644
--- a/Marlin/src/HAL/HAL_ESP32/HAL_timers_ESP32.h
+++ b/Marlin/src/HAL/HAL_ESP32/HAL_timers_ESP32.h
@@ -28,6 +28,12 @@
 #include <stdint.h>
 #include "driver/timer.h"

+// need this to pull I2S_STEPPER_STREAM definition from pins_*.h based on configured board
+#include "../../inc/MarlinConfig.h"
+// need to also include pins.h explicitly here as the above statement will be ignored if HAL_timers_ESP32.h 
+// is being included from inside MarlinConfig.h / HAL.h (but before pins.h)
+#include "../../pins/pins.h"
+
 // --------------------------------------------------------------------------
 // Defines
 // --------------------------------------------------------------------------
diff --git a/Marlin/src/pins/pins_ESP32.h b/Marlin/src/pins/pins_ESP32.h
index 9901dde20..dad1e0d81 100644
--- a/Marlin/src/pins/pins_ESP32.h
+++ b/Marlin/src/pins/pins_ESP32.h
@@ -40,6 +40,8 @@
 //
 // Steppers
 //
+#define I2S_STEPPER_STREAM
+
 #define X_STEP_PIN         128
 #define X_DIR_PIN          129
 #define X_ENABLE_PIN       130
vivian-ng commented 5 years ago

@felixstorm One suggestion is to create a separate pins_xxxxx.h file for your current setup. This way, the pins_ESP32.h can be the standard pins file for all boards based on the ESP32 (like pins_RAMPS.h) and every adaptation of it is a separate pins file.

Also, I am curious as to how you can connect the Ender-3's LCD to this setup. I am trying to figure out how the LCD's pins work... So far, what I can glean out for the Creality v1.1.2 board's LCD connector: Pin 1: VCC 5V Pin 2: LCD_PINS_ENABLE (connected to ATMEGA1284's SDA pin) Pin 3: LCD_PINS_D4 Pin 4: RESET (is this the EN pin of the ESP32?) Pin 5: BTN_ENC (connected to ATMEGA1284's SCL pin) Pin 6: BEEPER_PIN Pin 7: BTN_EN1 Pin 8: BTN_EN2 Pin 9: LCD_PINS_RS Pin 10: GND I guess for the ESP32, most of the pins to use this LCD display can be the freed-up pins (IO25, IO26, etc.) but just wondering if LCD_PINS_ENABLE must be SDA and BTN_ENC must be SCL?

felixstorm commented 5 years ago

@vivian-ng Yes, I also did create my own board and pins_ESP32_FS_ENDER3.h file (https://github.com/felixstorm/Marlin/blob/Felix_Ender3_ESP32_2.0.x/Marlin/src/pins/pins_ESP32_FS_ENDER3.h), but since pins_ESP32.h also references stepper pins above 128 which need I2S, I thought it would be helpful to have it in pins_ESP32.h as well.

And for the LCD on the Ender-3 take a look at the attached PDF. This is my current setup (R160 & R161 are not populated, based on @simon-jouet's work in his r2 branch) which works nicely on my Ender-3: 201904_Esp32ReprapController.pdf

Marlin needs a few more fixes for ESP32 to work with the current bugfix-2.0.x branch, but I will hopefully be able to file the corresponding pull requests within the next few days: https://github.com/felixstorm/Marlin/commit/6d3199539e919592f7f4926ab2c75e227cca6d1c https://github.com/felixstorm/Marlin/commit/01011ac973846a3e5b46db92b98409621a3e261d https://github.com/felixstorm/Marlin/commit/6281e378af18b5389db730edb92bfff250ea2e1e

felixstorm commented 5 years ago

@vivian-ng Sorry, did not read your question fully before: all LCD pins (including LCD_PINS_ENABLE and BTN_ENC) seem to be general GPIOs, I selected them more or less randomly and it works in my setup.

simon-jouet commented 5 years ago

Where did you define I2S_STEPPER_STREAM?

I just double checked, I was wrong in my previous reply. I have it set in Configuration_adv.h

The I2S path really seems a very clever (and cheap) way to go to get more GPOs and perfect stepper timing. I really do not know either why people do not use it more often.

I agree I think it's a good enough solution (it's not perfect considering the steps lag behind based on the buffer length) but it provides a very very accurate and fast timing at very low cost and very significantly reduces the number of interrupts and obviously free a lot of ports on the ESP32.

but they go up and down permanently between 5 and 8 °C. Do you remember if you had stable temperature readings on your printer?

It was much much more stable than that for me, still oscillating but far less than that. Unfortunately everything is disassembled at the moment (I really really need to spend some time to make that R2 board...) so I can't give it a try. I've seen recently quite a few reports (not ESP32 related) about the temperature oscillating so that might also be related? for the revision of your ESP you should see it when a new firmware is flashed.

And for the LCD on the Ender-3 take a look at the attached PDF. This is my current setup (R160 & R161 are not populated, based on @simon-jouet's work in his r2 branch) which works nicely on my Ender-3: 201904_Esp32ReprapController.pdf

Nice schematic, bare minimum for it to work which is perfect. I'm trying to do the R2 with a bit more stuff to be able to try new things but if any manufacturer were to make a production board they would go for a more basic feature like what you have here. On the R2 i'm also trying to keep both SPI free so the SD card and other SPI peripherals (screen, TMC2130) can run without clashing.

vivian-ng commented 5 years ago

@felixstorm Thank you for sharing your schematic. I am also trying to update my ESP32-based board to use I2S based on @simon-jouet's R2 design, and was trying to figure out how to use the additional pins for LCD. I figured there won't be enough pins for the normal RepRap smart display controller, but the 10-pin Creality LCD display might work.

Can I just check that the pinout for your LCD connector in your schematic is a plug-and-play for the Creality display? It seems different from the Creality schematic for their v1.1.2 board, but I am guessing the Creality schematic is unreliable... at least in the diagram; the pin numbers seem to match yours, if laid out in a 2x5 counterclockwise layout.

vivian-ng commented 5 years ago

@simon-jouet @felixstorm BTW, one suggestion about the code. I would suggest moving the #define statements for I2S_DATA, I2S_BCK, and I2S_WS into the pins definition file. Having it more or less hard-coded into the HAL (I2S.h) is kind of restrictive for future boards based on the ESP32.

felixstorm commented 5 years ago

@vivian-ng Yes, the LCD connector is plug-and-play for the stock Ender-3 LCD. I think I got the pin-out from here (but had to re-number the pins as said in the schematic) and then checked the Marlin source code to get the real pin functions based an their pin number on the stock MCU: https://github.com/RudolphRiedel/CR-10_wiring/blob/master/Ender3_schematic.PDF

Here as a screenshot from my layout viewed from the top of the board that contains the pin numbers and signal names: image

felixstorm commented 5 years ago

@vivian-ng Yes, moving the #defines for I2S to pin_*.h sounds like a good idea if they're currently inside i2s.h. I will hopefully be able to add that when I do the pull requests - maybe I even implement two different example boards, one with I2S and one without.

vivian-ng commented 5 years ago

@felixstorm Thank you for the schematic. Did you have any issues feeding back inputs to the ESP32 with the LCD controller? Because the LCD controller is being powered by 5V, but the ESP32 only accepts 3.3V inputs. My main worry is blowing out the pins on the ESP32 from the inputs like BTN_EN1 and BTN_EN2.

felixstorm commented 5 years ago

@vivian-ng No, voltage levels seem to be no problem. The encoder and button inputs are anyway only connected to ground without any external pullup (pullup is done inside Marlin/MCU) and the LCD has inputs only and works fine with 3.3 volt levels. And the reset pin does not seem to be connected at all on the LCD board - on some boards it seems to be used as an emergency stop button (but would then also be connected to ground only).

felixstorm commented 5 years ago

@simon-jouet Thanks for your information. I did run 20 autotune PID cycles again, but the temperature is still oscillating quite a bit (e.g. from 195 to 204 °C). I also powered the board just from USB to make sure I have a stable 5V supply, but still no change.
For now I have solved the practical problems by increasing TEMP_HYSTERESIS and TEMP_WINDOW and if I find some time I'll take a look at the code to see if I find an easy solution to do multisampling (as recommended even by Espressif: https://docs.espressif.com/projects/esp-idf/en/latest/api-reference/peripherals/adc.html#minimizing-noise).

@simon-jouet Since my questions have been answered in full detail (thanks again for that), shall I close this issue or leave it open to be seen more easily by other readers?

edsonbrusque commented 5 years ago

Hello @felixstorm.

I'm interested about your ADC noise.

It would be useful to know if the variation is in your readings or in your actual bed temperature. Are you seeing this variation only on the display? Do you have any means to measure the actual nozzle and bed temperatures (a thermometer with a logger) or the voltage at the ADC pins (an oscilloscope)?

If the temperature variation on the LCD is very quick (like changing +/- 5% all over the place) it's probably noise on the ADC pin.

If it's a more sinusoidal variation with a more or less defined duration (like going from 195% to 205% in 10 seconds and back) it would suggest poor PID tuning.

A quick way to filter noise is using larger capacitors (C30, C31 and C32 in your schematic). 10k and 100nF (as in your schematic) give a time constant of 1 us, which is usually adequate regarding high-frequency noise.

You also can't make this time constant very large (like changing to 100 uF capacitors that will give you one entire second time constant) as the PID would probably overreact and have slow and large oscillations).

Maybe you can post a quick video showing this temperature variation?

I have an Ender-3 and just learn about this ESP32Controller project. I would like to build it. Maybe it's time to order some stepper drivers... :)

felixstorm commented 5 years ago

@edsonbrusque I still have not figured out what exactly causes the noise in the temperature readings, but due to your question I started to dig again today. Unfortunately I still do not have an answer, but I have a few findings:

Hopefully I will have some more time tomorrow and then I'll try to check wire by wire what causes the noise.

BTW: The capacitors used for the thermistors are C20 and C21 which are already 4.7 uF (and adding even larger ones did not make a difference either).

edsonbrusque commented 5 years ago

Hello @felixstorm. It seems to me that the stepper drivers are injecting noise on your VCC (+3V3).

Is it possible to power the drivers from the +5V line?

Or can you have a separate regulator from 5V to 3V3 to power just the drivers?

The development boards don't have beefy capacitors and power regulators. I would rely on it just to power the components on the development board itself. You may even need larger capacitors (like 2 x 470uF x 16V) on the development board 3V3 just to lower the spikes caused by WiFi communication.

Maybe some bigger capacitors on the 3V3 and 5V power lines (on strategic places around the board) would help. I see that you have capacitor for the +24V line close to the drivers, but there's no capacitors for the VCC line. I would not assume that the (cheap?) stepper driver modules have adequate decoupling.

Which stepper drivers are you using?

Can you provide the board design file?

felixstorm commented 5 years ago

It turned out for the noise on the temperature readings to be caused by the I2S lines in combination with my quick-and-dirty board layout. I had expected the whole I2S path up to the drivers to fail, but not for it to have unwanted side effects on other areas. But 2 MHz is more than I usually deal with ;-)

As I anyway had the plan to update my board layout a bit, I will do that and post my findings here.

@edsonbrusque I had already completely removed the stepper drivers for the tests and even the shift register ics. I also did try to attach different kinds of capacitors to the different power lines. WiFi is off currently (primarily to reduce upload and Marlin boot time). The drivers I use are FYSETC TMC2208.

Felix

felixstorm commented 5 years ago

Finally I seem to have been successful. The third revision of my board works nicely and I do not seem to have any unusual noise in the temperature readings anymore.
Details including board layout can be found here.

Thanks again for everybody's help!