Skip to content
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

Fix emergency parser for stm32f1 #21011

Conversation

arjanmels
Copy link
Contributor

Description

Fixes emergency parser support for stm32f1 when using USB Composite Serial and adds emergency parser support when using USB Serial.

Requirements

Board using stm32f1. (Tested using Bigtreetech SKR Mini-E3 v1.2 with both USB Composite Serial and USB Serial.)

Benefits

Adds support for emergency parser when using USB Serial and fixes support for USB Composite Serial.
The original changes (#19279, #19281) do not work (for me). The function MarlinCompositeSerial.peek() return multiple times the same character in:

emergency_parser.update(MarlinCompositeSerial.emergency_state, MarlinCompositeSerial.peek());

Configurations

Use SKR Mini-E3 v1.2 example config

#define EMERGENCY_PARSER

For USB Composite Serial:
#define USE_USB_COMPOSITE

Related Issues

Original implementation: #19279, #19281, Related issues: #19623

@rhapsodyv
Copy link
Member

rhapsodyv commented Feb 6, 2021

Thanks for the fix. I’m the original author. But I don’t have a F1 board with usb serial to test. So I just did a blind work.
After some research, I saw that my error was that we can’t run before the RX callback, because is that callback that fills the buffer (used by peek).
To fix it properly, we would need to copy the original Rx callback from maple to marlin and edit it.
But I didn’t go further with it.

If your fix works, it’s great.
I’m just a little concerned by the fact that the original RX callback will not be called. I thought it was needed.

@arjanmels
Copy link
Contributor Author

I use a different hook (composite_cdcacm_set_hooks). The only thing this hook does is look for a special reset sequence (LEAF). So this functionality is not present anymore (but not so relevant in my view as M997 achieves the same on Marlin).

(I cannot call the original hook as it is not accessible (static) and no way to get its address. Workaround would be to copy the full hook functionality.)

@rhapsodyv
Copy link
Member

Ok. It's good if we don't need that rx callback :-)

The only thing we must make sure is that normal serial works while EP is enabled.

But your fix looks good to me.

@qwewer0
Copy link
Contributor

qwewer0 commented Feb 6, 2021

RAM & Flash report:

STM32F103RC_btt_512K
Before:
RAM:   [===       ]  29.4% (used 14472 bytes from 49152 bytes)
Flash: [====      ]  44.9% (used 235388 bytes from 524288 bytes)

After:
RAM:   [===       ]  29.4% (used 14472 bytes from 49152 bytes)		+0 bytes
Flash: [====      ]  45.0% (used 235852 bytes from 524288 bytes)	+464 bytes

STM32F103RC_btt_512K_USB
Before:
RAM:   [===       ]  33.8% (used 16600 bytes from 49152 bytes)
Flash: [=====     ]  46.6% (used 244548 bytes from 524288 bytes)

After:
RAM:   [===       ]  33.8% (used 16592 bytes from 49152 bytes)		-8 bytes
Flash: [=====     ]  46.7% (used 244604 bytes from 524288 bytes)	+56 bytes

Configuration.zip

@arjanmels
Copy link
Contributor Author

The only thing we must make sure is that normal serial works while EP is enabled.
It works for me, but more testing is always better!

@arjanmels
Copy link
Contributor Author

@thinkyhead Thank you for the cleanups. Look good to me.

@thinkyhead thinkyhead merged commit 1e726fe into MarlinFirmware:bugfix-2.0.x Feb 8, 2021
@oldman4U
Copy link

oldman4U commented Feb 8, 2021

Hi.
Sorry to bother you. I saw your PR and thought that it eventually would help us to fix a problem with executing the LOAD function using the touchscreen mode on a BTT TFT. The reason I ask you is, that after testing several mainboard, TFT's and different firmware back to November last year it seems, that only STM32 based mainboards are affected. In detail E3 mini as well as SKP Pro v1.1 and SKR E3 DIP which I personally tested. LPC based mainboards like my SKR v1.4 are working perfectly using the same firmware with the same settings.

I installed Marlin with your fix and now the screen freezes, before my printer crashed as soon as I pressed the Purge More button, like shown in this video from another user: https://youtu.be/kq4-bfFMH9I

It would be great if you could help

THANK YOU

Marlin config attached
Archive.zip

@thisiskeithb
Copy link
Member

@oldman4U We prefer not to handle user-support questions here. (As noted on this page.) For best results getting help with configuration and troubleshooting, please use the following resources:

After seeking help from the community, if the consensus points to a bug in Marlin, then you should post a bug report.

@arjanmels
Copy link
Contributor Author

@oldman4U you can reach me at github@mels.email to continue this conversation.

@oldman4U
Copy link

oldman4U commented Feb 9, 2021

Hi Keith.

Please take one minute to check the BTT Touchscreen firmware page to see, that (together with a stale bot and some other persons there) I tried to make the repository useful again, after having more than 250 open tickets.

Thank you

rmpel added a commit to rmpel/Marlin that referenced this pull request Feb 11, 2021
* bugfix-2.0.x: (177 commits)
  [cron] Bump distribution date (2021-02-11)
  chmod and paths
  [cron] Bump distribution date (2021-02-10)
  Reheat bed first
  Ender 3 V2 DWIN cleanup (MarlinFirmware#21026)
  Update M808 comment
  MAX Thermocouples rework (MarlinFirmware#20447)
  [cron] Bump distribution date (2021-02-09)
  Serial refactor. Default 8-bit ECHO to int, not char (MarlinFirmware#20985)
  Fix STM32F1 emergency parser (MarlinFirmware#21011)
  Allow SERVO0_PIN override on Creality Melzi (MarlinFirmware#21007)
  Fix animated boot screen
  Fix: Unsupported use of %f in printf (MarlinFirmware#21001)
  Fix mini12864 v2.1 + PSU control + NeoPixel backlight (MarlinFirmware#21021)
  [cron] Bump distribution date (2021-02-08)
  Fix LVGL "more" menu user items (MarlinFirmware#21004)
  Fix TEMP_0_TR_ENABLE, rename temp conditions (MarlinFirmware#21016)
  Fix ESP32 I2S init placement (MarlinFirmware#21019)
  Improve RPi host kernel panic mitigation
  Melzi, comments cleanup
  ...
rmpel added a commit to rmpel/Marlin that referenced this pull request Feb 13, 2021
…rmpel/Marlin into rmpel/bugfix-2.0.x/ender-3-skr-14-turbo

* 'rmpel/bugfix-2.0.x/ender-3-skr-14-turbo' of github.com:rmpel/Marlin: (177 commits)
  [cron] Bump distribution date (2021-02-11)
  chmod and paths
  [cron] Bump distribution date (2021-02-10)
  Reheat bed first
  Ender 3 V2 DWIN cleanup (MarlinFirmware#21026)
  Update M808 comment
  MAX Thermocouples rework (MarlinFirmware#20447)
  [cron] Bump distribution date (2021-02-09)
  Serial refactor. Default 8-bit ECHO to int, not char (MarlinFirmware#20985)
  Fix STM32F1 emergency parser (MarlinFirmware#21011)
  Allow SERVO0_PIN override on Creality Melzi (MarlinFirmware#21007)
  Fix animated boot screen
  Fix: Unsupported use of %f in printf (MarlinFirmware#21001)
  Fix mini12864 v2.1 + PSU control + NeoPixel backlight (MarlinFirmware#21021)
  [cron] Bump distribution date (2021-02-08)
  Fix LVGL "more" menu user items (MarlinFirmware#21004)
  Fix TEMP_0_TR_ENABLE, rename temp conditions (MarlinFirmware#21016)
  Fix ESP32 I2S init placement (MarlinFirmware#21019)
  Improve RPi host kernel panic mitigation
  Melzi, comments cleanup
  ...
rmpel added a commit to rmpel/Marlin that referenced this pull request Feb 13, 2021
* bugfix-2.0.x: (177 commits)
  [cron] Bump distribution date (2021-02-11)
  chmod and paths
  [cron] Bump distribution date (2021-02-10)
  Reheat bed first
  Ender 3 V2 DWIN cleanup (MarlinFirmware#21026)
  Update M808 comment
  MAX Thermocouples rework (MarlinFirmware#20447)
  [cron] Bump distribution date (2021-02-09)
  Serial refactor. Default 8-bit ECHO to int, not char (MarlinFirmware#20985)
  Fix STM32F1 emergency parser (MarlinFirmware#21011)
  Allow SERVO0_PIN override on Creality Melzi (MarlinFirmware#21007)
  Fix animated boot screen
  Fix: Unsupported use of %f in printf (MarlinFirmware#21001)
  Fix mini12864 v2.1 + PSU control + NeoPixel backlight (MarlinFirmware#21021)
  [cron] Bump distribution date (2021-02-08)
  Fix LVGL "more" menu user items (MarlinFirmware#21004)
  Fix TEMP_0_TR_ENABLE, rename temp conditions (MarlinFirmware#21016)
  Fix ESP32 I2S init placement (MarlinFirmware#21019)
  Improve RPi host kernel panic mitigation
  Melzi, comments cleanup
  ...
@sarvenn
Copy link

sarvenn commented Feb 13, 2021

@thisiskeithb all the stm32 based board restarts after selecting purge more in TFT mode. There are a couple of issues in BTT Touch screen firmware repository. Please have a look. Maybe it needs a fix from the Marlin side, I don't know.

@rhapsodyv
Copy link
Member

@thisiskeithb all the stm32 based board restarts after selecting purge more in TFT mode. There are a couple of issues in BTT Touch screen firmware repository. Please have a look. Maybe it needs a fix from the Marlin side, I don't know.

Open a new issue about that and give more details and configs.

@oldman4U
Copy link

I would like to add that the crash can be forced without any TFT attached by sending gCode commands via serial. It is my feeling that it is not Marlin but STM32 library related because everything works fine using the same commands on LPC based boards. So not sure where to start.

vyacheslav-shubin pushed a commit to vyacheslav-shubin/Marlin that referenced this pull request Mar 10, 2021
W4tel-BiDi pushed a commit to W4tel-BiDi/Marlin that referenced this pull request Apr 5, 2021
thinkyhead pushed a commit that referenced this pull request Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants