-
Notifications
You must be signed in to change notification settings - Fork 29
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
I2S motor and FET issues #10
Comments
Do you see it for the other steppers too are just X ? Because all the steppers are part of the same stream so I would assume that if it's something to do with I2S then all steppers would be jittering not just the X axis. I have seen something similar to this in my first ESP32 prototype (without I2s) where the noise was triggering the stepper drivers when it shouldn't. If you have an oscilloscope put a probe on the step pin of the X driver to start with and check if you just get a peak or not. Then you can put a probe on SDI/WCLK/BCLK to see if you also see some noise.
IIIRC the fan in Marlin is a PWM output so it won't work over I2S as is. Considering the FAN speed is part of the planner block it might be possible to simulate a PWM signal using the I2S stream but that's not implemented yet.
This points to roughly the same thing as the first point with the stepper moving just because of the noise. I will see how it looks with my new board hopefully the noise isn't too bad. What you can try in the short term is to reduce the noise of your board. If you have some aluminium or copper tape you can add some on top of the traces and create a grounds plane without have to reroute everything. Won't be pretty but at least you might be able to debug what's wrong.
This can be configured but that's about it yes. In the current configuration the lower 16bits are buffered in the I2S DMA stream and are perfect for time critical operation (with a bit of latency), all the operations in this stream are scheduled every time the DMA buffers finishes a block. The higher 16bits are a constant I2S channel value which is read from an I2S register so this will change as soon as the value is written. However, the constant channel is not very accurate timing wise because it will happen at the next word transmission of the I2S depending on when the register was written too, so if you write too fast some data might never have been sent. |
The Motor drift was on all 3 Axis in my Delta configuration after a Z-move - looks like the last move was somehow sticky. I checked this with a scope on BCLK and SDI - there was a pulse every 32ms - and even after motor disable timeout the enable was still there for 4 BCLK's with a pulse within. And i still have sometimes strange null output after 4ms - considering 250khz BCLK rate there might be something wrong every 1k/4k samples?!? Noise might be an issue with my current design - we should consider some serial termination resistors to improve signal integrity. But Noise shouldn't be that regular - the error condition stays forever with constant frequency. Temperatures are quite stable (using 22uf caps instead of 10us) - but having 26°C when breaking the wire won't trigger an exception. Maybe some voltage divider might give some more headroom for low temps. BTW - got the FETs for heating up at I2S Output >16 .... but still having problems with the fan. Maybe because the HAL tells Marlin that there are no PWM Outputs above 34 |
For PWM implementation on I2S, will this library be good reference? As for temperature readings, I will try out with a 22uF capacitor to see whether I get more stable readings. Regarding noise affecting the stepper motors, I didn't really notice on Y and Z axis, but Z is on a lead screw so any small stray steps may be too small to notice, which Y is a bed and the stray steps may not create enough torque in the first place. Will try to hunt down some copper tape... |
I've got a faint memory of a potential issue when the I2S data stream was set high for the last word of the buffer (because then there was no buffer afterwards to set it low). Might be something like that, might be a bit hard to replicate. Every 32ms would map to that, each buffer is 4k in size and therefore contains 1k samples. At 250khz the 32ms would be the 8 buffers in the dma repeating again and again when there are no movements. I will have a better idea when I get my board (and if i can replicate it)
Marlin expects hardware PWM, and doing PWM over I2S would effectively be software.
Potentially, although coding the PWM over I2S is fairly straightforward just needs some time. If you want a very basic PWM over i2s you just need to set x out of 256 bit high when scheduling the stepper steps. x in that case will be the duty cycle. |
The drift issue should be fixed if the DMA buffer is filled completely like Ringel/Marlin@9a50f1f @simon-jouet do you still have the 16Bit DMA/16Bit Register configuration? Only found a 16Bit-Only in esp32-i2s and the 32Bit DMA in esp32-i2s-dma |
@vivian-ng I personally use I2S pins for fans, but only for the controller and the hotend fans, so no PWM here. But when browsing the code to find out how these fans work, I did stumble across the define @Ringel Thanks for your fix. I will try it out when I do my next prints - unfortunately I do not have much spare time at the moment. |
@Ringel Thanks looks good, I don't remember well enough right now why this was done this way, but using a while looks cleaner anyway. From the looks of it I omitted that at the end of the moves
I thought it was the default config in Marlin that I had pushed but it doesn't look like it. @vivian-ng sorry about that I might have told you the wrong thing before. I will have to check my sample code see if I still have it somewhere. IIRC you need to change tx_chan_mod to 3 or 4 (https://www.espressif.com/sites/default/files/documentation/esp32_technical_reference_manual_en.pdf chapter 12) and write to I2S0.conf_single_data
That's good news, coding it wouldn't have been too bad but if it's built in it's even better |
@felixstorm Have you tried to see if FAN_SOFT_PWM would work for the part cooling fan as well running on I2S? I believer PWM values can be used for CONTROLLER_FAN and E0_AUTO_FAN, have you set them to some value other than 255 to see if there is a difference when FAN_SOFT_PWM is enabled? I will also try to find time to test this out. For soft PWM to work on I2S pins, there will be a need to change HAL.cpp so that the check for PWM pins is |
Yes - and if idle there are 4x stepper_pulse_phase_isr (counted but not pushed something) that caused the remaining 4 samples in the buffer to stick.
4 did the trick - see Ringel/Marlin@cdf3b84 for working split DMA/register writes. I first checked if the Display now works from the i2s extender ... unfortunately the LiquidCrystal library used digitaWrites instead of Marlin HAL. Getting 6 precious IOs back from the esp might be worth the fix ;)
FAN_SOFT_PWM works for the part fan ... E?_Auto_FAN an CONTROLLER_FAN still needs hardware PWM
Tried that - but returning PWM feature expects hardware PWM (at least done by HAL) and does not switch to software PWM |
@vivian-ng No, I have not tried it myself yet as my part cooling fan is connect to a direct GPIO. @Ringel Ok - I had only looked for the part cooling fan. But what does work for me is to just run them on full speed from the I2S outputs as both will switch to a normal |
Yes - but i'm used to reduce the fan noise of all fans. For that i've changed PWM from ledc-hardware to software timer, to be able to handle direct and register-i2s outputs in the HAL with the following changeset: Ringel/Marlin@8515126 Hardware-PWM might be more ressource friendly - but was double used by the servo HAL anyhow. Running PWM on the DMA outputs would also be nice ... but i'think i'll first try to get the display via the i2s register-pins up. |
Quick recap: My "to test":
@Ringel Will you be pushing those changes to the main Marlin repo anytime soon? Even if LCD is not working, any improvements to the current HAL would be great. Getting LCD to work would be great since it will free up at least 3 pins (LCD_ENABLE, LCD_RS, LCD_D4). Also, has anyone tried using 74HCT595 (TTL version)? Does it really make such a difference? |
Yeah pushing the fix for the i2s stream would be great. However, I don't think the constant channel for the top 16 bits should be pushed. That will be board dependent, if we want this we should probably set it behind #define so we can change from one mode to another |
You're right - added I2S_STEPPER_SPLIT_STREAM to enable. |
Thanks @Ringel, I think my only comment on your MR is that it replaces all ledc with the timer which is not something some people might want (for instance no I2S or I2S but no constant channel). Another reason why I'm not a big fan of this is that the ledc PWM is significantly more efficient than timer based and timers do cause problems with the WiFi (which is why I'm trying to use them as least as possible). I've got a PoC of switching the temperature ISR to a task in order to remove the need for all timers. EDIT: thinking about it we probably should not use the timer for ESP32's pins, ledc can be attached to any of the ESP32 pin so the logic here is only needed to do PWM over I2S. The logic you have could potentially be just in a #define I2S_TIMER_PWM ? I think a sanity check will be required to have I2S_TIMER_PWM only valid if I2S_STEPPER_SPLIT_STREAM is enabled. And i think remove the iteration in the analogWrite would be quite beneficial |
Having FAN_SOFT_PWM for all fans could remove need for this too. |
Yeah, can't remember who added the servo code, but we should probably review/refactor the existing code so there is no conflcit with the ledc channels |
@Ringel I agree that having @Ringel, @simon-jouet I am not so much worried about PWM and servos interfering as I purposely starting allocating pwm channels for servos from channels 12 up only. That leaves room for 12 fans, RGB channels etc. before there would be a conflict (and will still support 4 servos). |
@felixstorm yeah that makes sense, it might be good to add a check in analogWrite() just for sanity? Just realised something going through the code, I never pushed my improved temperature code. In the current code the attenuation is always at 11DB so we are losing in precision. I had modified this code to automatically shift the attenuation to have more precision ... I will have to check where I have that and probably modify it to cope with the upstream modifications... (that would also explain why people were saying they had big temperature delta while I didn't - noise was just part of the problem) |
I couldn't find the temperature code I wrote before so just rewrote it. Gave it a test on one of the R1 board and it's working fine. This should give much better temperature measurement (still doesn't solve the ~25degree min temp) @vivian-ng might be good if you could try considering you're the one with the biggest temperature variation. It would be useful to know which revision of the ESP32 you're running, it should say in platformIO when you upload e.g Chip is ESP32D0WDQ6 (revision 1) @felixstorm, @Ringel if you could try as well that would be great! The code is on my Marlin repo https://github.com/simon-jouet/Marlin/commits/esp32 (latest bugfix-2.0.x as of tonight) On a side note it anybody having issues with OTA? it's not working for me (it used to work fine). I also tried the WebSocket serial, and I don't know what changed but it barely works now, M114 works but M115 looks to be crashing the socket. |
@simon-jouet Okay, will download your HAL.cpp and give it a try as soon as I can. A bit busy with other work lately. I am actually using @luc-github's Marlin fork, which allows use of his ESP3D webUI. He has been working hard on making sure it is in sync with the main Marlin repo and all the updates there, and recently has been asking about how his work can be integrated back into the main repo. There will probably be some work to delete/add the changes in your HAL.cpp to fit into the one that luc has; hopefully not that much work so that I can get down to testing earlier. Side note: luc's Marlin fork has OTA working fine, and the webUI (which, in addition to a serial terminal for sending commands to the ESP32, has other buttons and such for moves, heating, fan control, file upload, etc.) has been tested to be working more or less fine. Some minor issues here and there, such as the temperature graph not updating when the ESP32 is busy with heating. But overall, more than adequate. At least enough for me to use my first prototype board (no I2S) to be used without an LCD controller; just use a smart phone or PC.
Fully agree with the ledc timer issue. A #define I2S_TIMER_PWM will definitely help. Design-wise, I think a new feature does not necessarily replace an existing one if the existing feature works. Users then have the choice of sticking with the old feature, or hopping onto the new one. |
Luc's HAL is based on mine (and now the upstream Marlin) so a large chunk of the HAL code should be the same which should make bringing those changes easy. I've seen Luc's issue on Marlin but I think instead of entry point a large chunk of his changes could be brought directly in Marlin (for instance all the new gcode). It's probably just a matter or cherry picking the features one by one and creating (and reviewing) MRs on Marlin. Also my end goal (and Grownseed) when we wrote the web stuff was to potentially define a reasonable API in Marlin and then you can use whatever UI you want. Any web UI will need movement, file upload so most code can be brought in without requiring an entry point for the handlers. If entrypoints are required then it might be good to look at the lcd code who uses them already. |
@simon-jouet this is what I do in ESP3D since the begining - the ESP3D-WEUI is not part of fw and same is used for GRBL_ESP32, but some feature are not in UI like telnet / email or push notification / DHT support, etc.. Other people developed their own webUI already. and no need to modify ESP3D |
why I suggested an entry point is because some decision like using SPIFFS for EEPROM (which is strange for me) instead of EEPROM NVS (what I did and seems better if user want to use FAT instead of SPIFFS), or need to have better access to serial than existing one for bridging, etc, I did not want to break what have been done and as I wrote it is just a proposal / discussion to propose other access - if it is not in plan I am fine ^_^ |
I also think that NVS might make more sense than SPIFFS, we could always check with @Idorobots and change it if we agree. It's still early days of the ESP32 so it might be better to change things like this now than later. For the other things i'm not sure what the status is at the moment, some people have reported OTA and websocket working for them, they definitely used to work for me but I pulled the most recent version and it's not working (can't upload using OTA (might need to double check my config) and the websocket is very buggy)
I really don't know what the plan is for Marlin :), it might be good to have a small sample in your PR issue so @thinkyhead can have a look and give his opinion |
@simon-jouet, |
@simon-jouet @luc-github |
@luc-github I also saw your issue and also was quite disappointed that nobody reacted on it officially. Maybe you could try to address thinkyhead directly as apparently he has one of @simon-jouet's first-generation board himself (see MarlinFirmware/Marlin#14345 (comment)) and always was pretty fast merging my ESP32-related PRs. And if there still does not come any official reaction, we might just agree for your fork to be our internal "official" one then for now? Looks like everybody that is actually using Marlin on the ESP32 is taking part in this issue 😉 @simon-jouet I will definitely test your temperature code. I will very soon be out for vacation for 2 weeks or so but hopefully still make one larger print before. |
@luc-github we can probably bring those changes in the ESP32 branch, barely anybody uses that at the moment and if it's an improvement and stays within the ESP hal it should make it through. For the stepping there shouldn't be any jitter when i2s is used, I guess with the timer ISR it might be an issue. I remember having some issues dealing with the WiFi code when the timers were used. Having the Gcode to configure the WiFi built into marlin would clearly be useful, reflashing just for the SSID and password doesn't make much sense. (I wrote it that way because getting WiFi to work was very low prio in the original HAL) I think the entry might still make sense, depending on the web UI used some new endpoints might be required and adding a way to extend the http handlers might be useful (which is I think what you were saying in your FR?). Like I said in the previous comment it might be worth checking how the extensible lcd code works and use something similar? @Idorobots thanks, good to know :) should be easy to get that switched then! @felixstorm yeah thinkyhead has one of the R1 board and I will send him an R2 board once they are working (still waiting on the PCB and some components). From what I've seen he his quite pragmatic, if it's an improvement for Marlin that doesn't hinder anything existing. I don't know him though, so that's just my impression :). No particular connections no, same as everybody here, mostly github and very briefly on discord. The problem with the fork (which is why i'm trying to avoid that if possible) is that it's much more pain to maintain and it won't help new people/companies adopting the ESP32 which isn't really in our best interest. If we all agree on some things that are better for the ESP32 hal then I think it will be merged in (as long as it stays within the ESP32 HAL). If Marlin decides to stop supporting the ESP32 for whatever reason they can just delete the folder and call it done. All the current WiFi code is either from my initial MR or my brother (grownseed) and fixes from @Idorobots so it's quite limited and can be easily replaced with something else.
Thanks, curious to hear feedback. I need to tweak it a bit before opening an MR in Marlin but the current code should be fully operational. |
Please don't get me wrong - I'm not for a fork either. It was just meant as a last resort for some time until the Marlin maintainers e.g. have released 2.0 and have a bit more time again. |
@Idorobots, sorry I did not react when you pushed your PR as I am not watching Marlin git, I was overflowded with Marlin notifications and I unsuscribed a while ago, and still unwatch, I just react when my name is mentionned ^_^. @felixstorm, yes it is not my plan neither to maintain a long term Marlin's fork, I just did it to have full tested solution before proposing a PR to Marlin. I rebase on demand without issue until then. About @thinkyhead, I think he is busy and does not need a new reminder because @vivian-ng already did it and he is definitly aware of ESP3D since @grownseed PR as I was mentionned on it and answered, and he recently watched a youtube video about it and commented it. 😸. But you can do some lobbying , I am ok. ^_^ That said,sSorry @simon-jouet to hack your thread with such unrelated issue - but I happy to learn steppers are better with i2s |
@simon-jouet An almost 12 hour PETG print (that had a large layer shift on a previous attempt a week or two ago) finished nicely and without any problems on my ESP32 Ender-3 this morning (with I2S fix Ringel/Marlin@9a50f1f and with your new temperature code). Some stringing, but that's definitely neither Marlin's nor the ESP32's fault 😉. @luc-github I would just go ahead and file some small separate PRs - starting with the ones entirely inside the ESP32 HAL, e.g. one replacing EEPROM SPIFFS with NVS, another one with your synchronous WebServer implementation (as an alternative using I'll be off for vacation more or less today for almost 3 weeks, but I'll try to monitor the discussion on my mobile if I have a chance (just copy me on any new issues or PRs). And looking forward to coming back and trying out Marlin with ESP3D integrated - maybe already from https://github.com/MarlinFirmware/Marlin or from https://github.com/luc-github/Marlin Kind regards, |
@simon-jouet I have tested the new ADC code, without a thermistor connected and using 4.7K pullups, the reading was around 25 degC. With room temperature at around 28 degC, I get a reading of 30 degC when I connect a thermistor to my non-I2S board. With my I2S board, I get something like 35 degC instead. Temperature reading fluctuations on both boards seem to be around 1 to 2 degC, but I still needed to set PWM on I2S pins work. Or at least I think they work. I connected one of them to a LED, and at various output values, the LED changed in brightness. I2S pin can also be used for LCD's beeper. But no luck trying to get it to work with LCD_ENABLE, LCD_RS, or LCD_D4. Oh, trying to use 1K pullups for the thermistors doesn't really work too. At room temperature, with or without a thermistor connected, using a 1K pullup will give a temperature reading of 68 degC. |
Two separate issues:
Any suggestions on where to start looking to see how to fix these issues? |
@vivian-ng - unfortunately I do not have solutions, just my experience:
In addition to your findings I have some more nuisances:
But all in all these were just a bit annoying and not a real problem. And since unfortunately I currently have little spare time, I also was not able do research on what might cause the issues. |
|
@felixstorm
If anyone has any suggestion on how to test ESP32 with ILI9341 display, do let me know. I have a M5Stack which I want to use to test this. It has three buttons, which can be used to simulate buttons up, down, and enter. |
Has anyone tried using I2S on a coreXY setup? I am facing random resets during homing. The resets happen either when changing direction along the same axis (such as when it has reached the X endstop, and is reversing direction for X_HOME_BUMP_MM) or when changing axis (such as after X and Y axes have homed, and it is switching to move Z axis for homing). On a ESP32 board that does not use I2S (only native pins), I do not have such random resets on my coreXY setup. Also, on a cartesian printer, I do not observe such random resets when using I2S for the stepper stream, so I suspect it has to do with the combination of coreXY mechanics (which requires both X and Y motors to move and change directions) and the use of I2S stepper stream. Wondering if anyone else has experienced such an issue. |
Hi, i have same issue mentioned before in my post. İ tried raw cartesian mode and doesnt occur any brownout reset. But when i use i2s core xy browout reset occurs randomly when touching home switches. |
This is the error that triggered the reboot when homing using a coreXY setup.
This is the decoded stack:
I am not really sure what this means, but I do not experience such issues when not using I2S stepper stream on a coreXY. I2S stepper stream on a cartesian also works fine, so it seems the issue is with the planner and endstop during coreXY movements. Maybe possible problems with HAL_timers_ESP32.cpp? Seems like the HAL_timer_enable_interrupt and HAL_timer_disable_interrupt don't do anything.
Or possible problem with stepper.cpp? Because it says something like this:
I don't understand Marlin's way of planning the steppers enough, so it is tough for me to debug this on my own. |
I managed to solve the reset issue with luc's help. See details in the separate issue here and the solution here. @felixstorm You mentioned about the printer freezing in the middle of prints before continuing. I also experienced the same issue when I am using my board with I2S. It does not happen on my other board which does not use I2S. Basically, about 2-3 hours into a print, the printer will stop printing for a couple of minutes before continuing with the print. For long prints, this can happen several times. But the printer is still actively sending out PWM signals for heating (my heated bed and hotend continue to maintained at temperature) so I think the issue is with the I2S stepper stream, but not with the I2S in general. Maybe someone with better knowledge of the I2S stepper stream can advise... |
@vivian-ng My experiences are about the same, only that the pauses seem to be a bit shorter IIRC (maybe 30-60 seconds). Unfortunately I did not have any spare time to do research on this during the last months, but hopefully I'll find some time during the holiday season. Also, as I do not have as much CPP knowledge as @simon-jouet by far, I can only give it a try and maybe with a lot of luck might find a reason for the pauses... |
@felixstorm Are you using TMC drivers? I will be trying out a few longer prints over the next few days, using both I2S and non-I2S versions of my boards to see if it is a problem with I2S. Both will be using TMC2130 SPI mode on X and Y. My hunch is that it could be a TMC issue, due to similar symptom here. |
@vivian-ng Not really. I do have stepper driver modules on the PCB that use TMC drivers, but I do not use TMC support inside Marlin, so I do not think that our freeze is related to the TMC drivers. I also watched a few prints during the last few days and did start with a tiny bit of debugging, but unfortunately I did not find anything obvious that caught my attention. My findings so far (just for reference):
If I find a bit more time, I will try to continue debugging a bit more to trace down where the delay comes from, but as I am quite busy, this might not happen anytime soon :-( @vivian-ng, @luc-github Are both of you absolutely sure that it only happens when using I2S? This might still very well be correct, but then the problem does not seem to be directly related to the I2S code (as this continues to run just fine as listed above). |
@felixstorm I do not have a system setup actually - I just have r1 @simon-jouet's board so I never saw the issue. |
@felixstorm I can't say for sure, it is only a hunch. I just did a few prints over the past two days. 3 prints on non-I2S board running TMC2130 in SPI mode. Longest print was 6+ hours, no freeze. 2 prints on I2S board running A4988 and no LCD, longest print was 5+ hours, no freeze. I will try a few longer prints over the next few days to see if I get any freezes. |
Cross posting from here as it likely has to do with I2S stepper stream. Hopefully, this sheds some light on the babystepping issue on I2S stepper stream. Test machine: cartesian (Ender-3) Got an error during babystepping with the following:
Using babystepping by gcode gives the same error, minus the LCD portion of the stack. This is likely an I2S issue since I tried gcode babystepping on the MRR ESPA (non-I2S) and it works. Just posting in case anyone has time to look at the code and try to decipher what may be wrong with the timer code. |
Collective minds of the ESP32+Marlin community, two questions:
|
@vivian-ng I think the problem with both is that when using I2S, the "real" stepper ISR (see https://github.com/MarlinFirmware/Marlin/blob/1955eea1b8f13697259e0e2403d47279d2d1ac6a/Marlin/src/module/stepper.cpp#L1338) never gets called but only |
Going to close the issue now that the R3 board is out (it will need some tweaks) and so we can start issues that are more on topic Just to go back to the 5 months old comments above. Yeah @felixstorm is right when using the i2s stream we manually call the pulse_phase_isr just to get the timing of the next step to add to the buffer. To support linear advance we would need to do the logic there too, I haven't had a look at that code in a long time so I can't say just now how much effort it would be. For the baby stepping it's because the way it's working in Marlin is a bit hacky, it just creates a step outside of the planner so it doesn't get scheduled on the i2s stream |
As @simon-jouet mentioned in PR#6 about starting a new issue to discuss the I2S-related motor and FET issues, I have taken the liberty to open the issue.
I am also experiencing issues with the I2S stream for motors, similar to what @Ringel mentioned. Sometimes, after the motors have stopped, it will start to drift on its own (I see this on my X axis, which uses the pins 128, 129, and 130). I also had an experience where partway through a print, the X gantry just drifted toward X_MIN, and kept crashing into the endstop until I had to turn off the printer. It seemed random because I did the same print again and the print completed.
As for driving of FETs, I am able to drive my E0 MOSFET using pin 140, but I am not able to control my fan MOSFET using pin 141. I will check to see if using a pin from 144 and higher will give more consistent results.
I have horrible temperature reading fluctations of plus minus 5 degs, which could be due to noise from the I2S lines as mentioned by @felixstorm before. Will try to reroute the lines and add a ground plane below the I2S lines to see if it helps. Meanwhile, I had to set TEMP_HYSTERESIS as 10 to allow for printing to commence. Definitely not ideal, haha.
I haven't had any luck with trying to drive the LCD controllers via I2S, but will try to use the pins 144 and above to see if it works. If my understanding of what @simon-jouet is saying, the lower pins (128 to 143) should be used for the motors, while the higher pins (144 to 159) can be used for MOSFETs and LCD. Will give that a try.
The text was updated successfully, but these errors were encountered: