search

luc-github / ESP3D

FW for ESP8266/ESP8285/ESP32 used with 3D printer
GNU General Public License v3.0
1.74k stars 465 forks source link

[BUG] Freeze issues (Marlin M43, M122, on ESP3D 3.0) #995

Closed rondlh closed 7 months ago

rondlh commented 8 months ago

Describe the bug

ESP3D 3.0 seems to have an issue with TABS (ASCII 9), when I run a M122 (Marlin get TMC info, which returns some Tabs) then ESP3D freezes. I also see the same thing when using M43 (pin debug info, which responds with a lot of data (8-9KB). It sometimes works, and sometimes freezes ESP3D. Also the Marlin startup info can cause the same issue (also a lot of lines), when I restart Marlin after ESP3D is connected.

To Reproduce

  1. Connect to Marlin machine and run M122, M43, or restart Marlin when ESP3D is connected.
  2. M122 cause ESP3D to freeze 2 lines are shown on the terminal, M43 locks up within a few tries, some data is usually received.

Expected behavior No freezes

ESP3D Firmware:

Current Marlin build (Jan 2024). NOTE: ESP3D 2.1.x does not have this issue.

Board used:

Additional context Connection layout: Marin <--250K Baud--> BTT TFT35V3 <--1M Baud--> ESP3D

luc-github commented 7 months ago

if the ESP would report the time when the time is available (configurable in configuration.h perhaps).

That looks more reasonable than trying complex things not related to time - but you need a trigger in M117 message to know it is time , or do you have a command that ESP should send including time ?

Or do you use json format to query ? which would be is easier to parse and get all data properly

rondlh commented 7 months ago

be noted not everyone use the timestamp

Yes, of course. That's not a problem. In the TFT I just timestamp the notification if the time is available, otherwise I use relative print time to timestamp.

I don't have that "Show Sensors" option in the Settings/Charts, but I have an additional chart (without a label) in the Dashboard. Perhaps I disabled something in the configuration. Charts Charts2

What tool do you use to create the index.html?

I have an old CNC machine (CNC3018), running GRBL, it reports the laser temperature. This machine is quite popular in China. I think if you remove the space from the regular expression then both ways ("T:21.23 /25" and "T:21.23/25") will work.

luc-github commented 7 months ago

you are not using the GRBL version of index.html ? I have a CNC3018 - but did not know it reported temperature can you elaborate ?

I use vscode : https://esp3d.io/esp3d-webui/v3.x/documentation/compilation/index.html

rondlh commented 7 months ago

That looks more reasonable than trying complex things not related to time - but you need a trigger in M117 message to know it is time , or do you have a command that ESP should send including time ?

Or do you use json format to query ? which would be is easier to parse and get all data properly

I don't use json format. For the TFT is would be easiest to parse something like: IP:192.168.1.123 DATE:2024-02-05 17:09:41

Similar to how Marlin works, just add "T:" in front of the temp, or B: C: etc.

There are many DIY kits for the CNC3018, this one can monitor laser temperature and motor temperature. It actually waits for the laser to warm up before starting. I believe it runs GRBL 0.9?. I is probably customized.

luc-github commented 7 months ago

so you with grbl should use this webui not 3d printer one image

luc-github commented 7 months ago

About your request I have a proposal : if there is a setting PRINTER_HAS_DISPLAY in configuration.h, it will enable the command[ESP212]<text> In this text you will be able to use some variable like %ESP_IP% for local IP, %ESP_NAME%, for local name, '%ESP_DATETIME%' for ESP date time Also I will add a hook when IP is get to process a command and another one when time is get to process a another command, both defined in configuration.h, so you can use [ESP212]IP:%ESP_IP% that wil generate M117 IP:192.168.2.50 and [ESP212]DATE:%ESP_DATETIME% that will generate M117 DATE:2024-01-05 17:40:00

Does it fit your needs ?

But it will only work with 3D Printers because grbl does not support M117

rondlh commented 7 months ago

About your request I have a proposal : if there is a setting PRINTER_HAS_DISPLAY in configuration.h, it will enable the command[ESP212]<text> In this text you will be able to use some variable like %ESP_IP% for local IP, %ESP_NAME%, for local name, '%ESP_DATETIME%' for ESP date time Also I will add a hook when IP is get to process a command and another one when time is get to process a another command, both defined in configuration.h, so you can use [ESP212]IP:%ESP_IP% that wil generate M117 IP:192.168.2.50 and [ESP212]DATE:%ESP_DATETIME% that will generate M117 DATE:2024-01-05 17:40:00

Does it fit your needs ?

That would be fantastic, it's super flexible, and makes integration very easy, you have my vote :D

But it will only work with 3D Printers because grbl does not support M117

For the CNC system my only problem is that I need to modify the RegEx for the temperature readings to be read. The system doesn't have a display.

Do you have any idea why I have this extra empty chart that I cannot disable?

I'm not sure if you still maintain V2.1, but I noticed something odd. In syncwebserver.cpp '\r' is filtered out: current_buffer.replace("\r",""); I believe this operation should be moved outside of the while loop (3 lines above), it does not need to be repeated for every line ending on '\n'.

luc-github commented 7 months ago

That would be fantastic, it's super flexible, and makes integration very easy, you have my vote :D

Ok should be done in couple of days

For the CNC system my only problem is that I need to modify the RegEx for the temperature readings to be read

I still do not understand you mentionned you use grbl but you seems using 3D printer webui ... why ?

Do you have any idea why I have this extra empty chart that I cannot disable?

Nope, I cannot reproduce, may be it is linked to the way you use the webui or the preferences.json is corrupted - delete it

I'm not sure if you still maintain V2.1,

Yes only for critical bugs, all new features enhancement go to 3.0, optimization like you suggest are neither done anymore but checked on 3.0 instead

rondlh commented 7 months ago

That would be fantastic, it's super flexible, and makes integration very easy, you have my vote :D

Ok should be done in couple of days

For the CNC system my only problem is that I need to modify the RegEx for the temperature readings to be read

I still do not understand you mentionned you use grbl but you seems using 3D printer webui ... why ?

Yes, I should change that. I was just checking out the status of V3.0 and didn't look to closely into it.

Do you have any idea why I have this extra empty chart that I cannot disable?

Nope, I cannot reproduce, may be it is linked to the way you use the webui or the preferences.json is corrupted - delete it

Deleting the preferences did the trick, thanks!

luc-github commented 7 months ago

please give a try to this test branch : https://github.com/luc-github/ESP3D/tree/ESP214-and-hooks Command is [ESP212] to send to screen Configuration.h has the hooks enabled for IP and for DATETIME

On my side it looks ok image

rondlh commented 7 months ago

please give a try to this test branch : https://github.com/luc-github/ESP3D/tree/ESP214-and-hooks

That is fast, I certainly will, thanks.

I checked if if I could find the reason for my charts issues. I think I found a typo in Charts.js (should probably be 0, 1, 2, but is 0, 1, 1)

    charts[0].ref = useRef(null)
    charts[1].ref = useRef(null)
    charts[1].ref = useRef(null)

It seems the charts are supposed to have different colors, but currently they are all pink (bed and extruder).

No very important, but there are several mistakes in the comments for "chartColors" (Charts.js)

const chartColors = [
    "255,128,128", //pink
    "0,0,255", //dark blue
    "0,128,0", //dark green
    "198,165,0", //gold
    "255,0,0", //red
    "0,0,128", //blue
    "128,255,128", //light green
    "255,128,0", //orange
    "178,0,255", //purple
    "0,128,128", //green blue
    "128,128,0", //kaki
    "128,128,128", //grey
    "0,0,0", //purple
]
const chartColors = [
    "255,128,128", //pink
    "0,0,255", //blue
    "0,128,0", //dark green
    "198,165,0", //gold
    "255,0,0", //red
    "0,0,128", //dark blue
    "128,255,128", //light green
    "255,128,0", //orange
    "178,0,255", //purple
    "0,128,128", //green blue
    "128,128,0", //olive
    "128,128,128", //grey
    "0,0,0", //black
]
luc-github commented 7 months ago

I remember I did on purpose - the green color was not great alone so I kept the blue for first third chart and I did not spent time to optimize the color rendering to be honest

yes comment are copy and paste issues (fixed in code now)

luc-github commented 7 months ago

@rondlh [ESP112] is not the topic of this thread so please confirm it is ok by commenting https://github.com/luc-github/ESP3D/pull/999

About the freezing issue please confirm it is now fixed on your side as I was never able to duplicate it but only the data lost so we can close this ticket

rondlh commented 7 months ago

Confirmed, the freeze issue is fixed by patching "WebSocket_Server::isConnected()". The ESP3D response in the browser is now much faster and more consistent. I tested it on an ESP32 and ESP32S2. Thanks for the great support!

Should I close this topic and open a Feature Request for the "hooks"?

BTW: In esp3d/src/modules/notifications/notifications_service.cpp there is a typo on line 681, I guess it should be _token2 = ""; I enabled some compiler warnings, but to my surprise this issue is not pointed out.

luc-github commented 7 months ago

Confirmed, the freeze issue is fixed by patching "WebSocket_Server::isConnected()".

Cool then I close issue

Thanks for the great support!

Well you did most of the work and investigation - thanks to you ^_^

Should I close this topic and open a Feature Request for the "hooks"?

No need just comment on PR

BTW: In esp3d/src/modules/notifications/notifications_service.cpp there is a typo on line 681, I guess it should be _token2 = "";

Yes again a bad copy and paste - orz .... fixed now in coming PR thank you

I enabled some compiler warnings, but to my surprise this issue is not pointed out.

Because doing twice the same command is not an error it is more a logic error - and that will be only seen if you ask an IA to review the code which I do not do usually, may be I should but I lack of time to review all ^_^

github-actions[bot] commented 7 months 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.