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

✨🐛 HC32 - Add SERIAL_DMA, fix SDIO and MEATPACK #26845

Merged

Conversation

shadow578
Copy link
Contributor

Description

this PR adapts the HC32 HAL to make use of some new features in the arduino core and ddl, including support for SERIAL_DMA.
It also introduces some extra checks aimed to improve error messages.

Requirements

  • any supported HC32-based board.
  • updating the packages framework-arduino-hc32f46x and framework-hc32f46x-ddl to their latest release versions (1.1.0 and 2.2.1 respectively).
    • using a older version of framework-arduino-hc32f46x will cause an error prompting the user to update.

Benefits

  • general improvements in Serial communication* and support for SERIAL_DMA
  • automatic configuration of the arduino core based on Marlins Configuraton.h and Configuration_adv.h
    • e.g. enabling core debugging when enabling MARLIN_DEV_MODE
  • fixed support for meatpack

During my testing (about 10 hours of printing as of now), serial errors when host printing were reduced to 0%.

* see shadow578/framework-arduino-hc32f46x#10 for details on the 'general improvements' part

Configurations

sample configuration for Aquila X2 w/ DWIN MarlinUI + BLTouch + SERIAL_DMA

Related Issues

N/A

@classicrocker883
Copy link
Contributor

unrelated to this PR specifically, but HC32 related:
I have run into an issue after flashing the firmware I would get a continual restart loop while the SD card is inserted. once removed it can be used as normal, otherwise anytime the SD card is put in it reboots over and over. is that still an issue?

@shadow578
Copy link
Contributor Author

shadow578 commented Mar 6, 2024

unrelated to this PR specifically, but HC32 related: I have run into an issue after flashing the firmware I would get a continual restart loop while the SD card is inserted. once removed it can be used as normal, otherwise anytime the SD card is put in it reboots over and over. is that still an issue?

@classicrocker883 thanks for bringing this issue to my attention.
after looking into the sdio HAL, i've noticed that with every initialization of the SDIO (= every time a sd card is inserted), the card handle was leaked.
on repeated sd card insertions, this would cause the system to run out of memory and crash.

anyway, i've created a fix and added the commit to this PR (even if it's not related...)


EDIT:

just noticed that @classicrocker883's issue occurs on boot.
maybe increasing the available heap memory (using DDL_HEAP_SIZE) could help?

if not, it'd probably be best to move the discussion to a issue

@classicrocker883
Copy link
Contributor

just noticed that @classicrocker883's issue occurs on boot. maybe increasing the available heap memory (using DDL_HEAP_SIZE) could help?

if not, it'd probably be best to move the discussion to a issue

as for the value to change what do you suggest? in that link there are two different values presented, default and example. im not sure where to start.

@shadow578
Copy link
Contributor Author

as for the value to change what do you suggest? in that link there are two different values presented, default and example. im not sure where to start.

the default values are what you'd be using right now, so i'd suggest setting them like so:

[HC32F460_base]
# (...)
build_flags      =
  -D DDL_STACK_SIZE=0x800 -D DDL_HEAP_SIZE=0x2000
  -D ARDUINO_ARCH_HC32
  -D PLATFORM_M997_SUPPORT
# (...)

when everything worked, you should see that the amount of free memory that marlin prints on start increased by ~5120 bytes.
so when before you saw "echo: Free Memory: 1800" before the change, after you should see "echo: Free Memory: 6920".

please note that for this to do anything at all, you must use version 2.2.1 of the ddl.

@classicrocker883
Copy link
Contributor

classicrocker883 commented Apr 6, 2024

something isnt working with app_config.h, I had to revert hc32.ini and things at least compiled normally.

in that file, I see:

#if ENABLED(POSTMORTEM_DEBUGGING)
  // disable arduino core fault handler, as we define our own
  #define CORE_DISABLE_FAULT_HANDLER 1
#endif

yet this is automatically enabled/defined in hc32.ini

  -D CORE_DISABLE_FAULT_HANDLER             # Disable arduino core fault handler (we use our own)

should POSTMORTEM_DEBUGGING also be enabled or can -D CORE_DISABLE_FAULT_HANDLER be commented out?

@shadow578
Copy link
Contributor Author

@classicrocker883

something isnt working with app_config.h, I had to revert hc32.ini and things at least compiled normally.

make sure you're using the latest release (at least 1.1.0) of the arduino core.

in that file, I see:

#if ENABLED(POSTMORTEM_DEBUGGING)
// disable arduino core fault handler, as we define our own
#define CORE_DISABLE_FAULT_HANDLER 1
#endif

yet this is automatically enabled/defined in hc32.ini

-D CORE_DISABLE_FAULT_HANDLER # Disable arduino core fault handler (we use our own)

should POSTMORTEM_DEBUGGING also be enabled or can -D CORE_DISABLE_FAULT_HANDLER be commented out?

this is indeed a change in the default behaviour of the HC32 HAL which i honestly didn't really consider.
as per the fault handler documentation, CORE_DISABLE_FAULT_HANDLER will disable the handler built into the arduino core.

so the old behaviour was to just always remove the fault handler, meaning a fault would just lock up the system.
with the change, you'll always get a fault report (at the cost of some flash usage).

imo the new behaviour is a better default, as you'll be made aware of the issue.

@InsanityAutomation
Copy link
Contributor

@shadow578 Is this still in a good state in youre opinion? Was looking at getting the Tenlog HC32 board ported in soon and it does seem to call in some of the stuff done here in their fork so figured this would be a prereq.

@shadow578
Copy link
Contributor Author

@shadow578 Is this still in a good state in youre opinion? Was looking at getting the Tenlog HC32 board ported in soon and it does seem to call in some of the stuff done here in their fork so figured this would be a prereq.

as far as i'm concerned, this should be good to merge and thus in a good state for adapting to a different HC32 board.

but please note that in the end it shouldn't really matter what Tenlog's port "calls in", as the HAL should work for basically every HC32 board with only changes to the config (offset in hc32.ini and pins).
i've seen some people trying to port the HAL to different boards and in doing so trying to fork and modify the DDL and stuff. this should not be needed.

@shadow578
Copy link
Contributor Author

@thisiskeithb sorry for asking, but is there anything preventing the PR from getting reviewed / merged?

@thisiskeithb
Copy link
Member

sorry for asking, but is there anything preventing the PR from getting reviewed / merged?

We have nearly 100 open PRs. Please have some patience.

@InsanityAutomation
Copy link
Contributor

I'm not a bare metal side expert on the underlying platforms, so I requested Jason review. I don't see anything that jumps out as wrong to me, but he's the resident expert so I'll leave the review to him

ini/hc32.ini Show resolved Hide resolved
Marlin/src/HAL/HC32/MarlinHAL.cpp Show resolved Hide resolved
Marlin/src/HAL/HC32/MarlinSerial.cpp Show resolved Hide resolved
Marlin/src/HAL/HC32/app_config.h Show resolved Hide resolved
Marlin/src/HAL/HC32/sdio.cpp Show resolved Hide resolved
@sjasonsmith sjasonsmith merged commit dca6afc into MarlinFirmware:bugfix-2.1.x Apr 14, 2024
61 checks passed
@sjasonsmith sjasonsmith changed the title SERIAL_DMA support for HC32 ✨🐛 HC32 - Add SERIAL_DMA, fix SDIO and MEATPACK Apr 14, 2024
blu28 added a commit to blu28/Marlin-blu that referenced this pull request Apr 20, 2024
commit 02ba6f9
Author: thinkyhead <thinkyhead@users.noreply.github.com>
Date:   Fri Apr 19 00:21:25 2024 +0000

    [cron] Bump distribution date (2024-04-19)

commit dba0010
Author: Andrew <18502096+classicrocker883@users.noreply.github.com>
Date:   Thu Apr 18 19:04:03 2024 -0400

    🎨 Rename some G-code files (MarlinFirmware#26981)

commit 90667f6
Author: I3DBeeTech <129617321+I3DBeeTech@users.noreply.github.com>
Date:   Fri Apr 19 02:24:17 2024 +0530

    🐛 Fix BLACKBEEZMINI fan, info (MarlinFirmware#26983)

commit d6961b2
Author: thinkyhead <thinkyhead@users.noreply.github.com>
Date:   Wed Apr 17 06:06:51 2024 +0000

    [cron] Bump distribution date (2024-04-17)

commit 07ebb81
Author: Javlon Sodikov <5047093+javlonsodikov@users.noreply.github.com>
Date:   Wed Apr 17 10:25:22 2024 +0500

    🩹Fix ProUI error when !CASELIGHT_USES_BRIGHTNESS (MarlinFirmware#26976)

    * Fix the compile error with the case light menu

    Fix the compile error with the case light menu

    * Add failing test

    ---------

    Co-authored-by: Jason Smith <jason.inet@gmail.com>

commit 245db73
Author: thinkyhead <thinkyhead@users.noreply.github.com>
Date:   Tue Apr 16 18:06:16 2024 +0000

    [cron] Bump distribution date (2024-04-16)

commit 9342dae
Author: Scott Lahteine <thinkyhead@users.noreply.github.com>
Date:   Tue Apr 16 12:17:47 2024 -0500

    📝 Remove dead PDF links

commit 1f84f50
Author: thinkyhead <thinkyhead@users.noreply.github.com>
Date:   Mon Apr 15 02:38:10 2024 +0000

    [cron] Bump distribution date (2024-04-15)

commit 3326c74
Author: Scott Lahteine <thinkyhead@users.noreply.github.com>
Date:   Sun Apr 14 16:26:16 2024 -0500

    📝 Minor README changes

commit 0269106
Author: Scott Lahteine <thinkyhead@users.noreply.github.com>
Date:   Sun Apr 14 16:24:14 2024 -0500

    🎨 Dagoma D6 followup

commit 95d38a8
Author: Sophist <3001893+Sophist-UK@users.noreply.github.com>
Date:   Sun Apr 14 21:04:52 2024 +0100

    ✨ Add Dagoma D6 as found in DiscoUltimate v2 TMC (MarlinFirmware#26874)

    * Add Dagoma D6 board as used in their DiscoUltimate v2 TMC.

    Taken from the Dagoma fork of Marlin DU_MC branch where it is called FYSETC_DAGOMA_F5 and explicitly confirmed by Dagoma as being the D6:

    "the BOARD_FYSETC_DAGOMA_F5 is effectively the definition for the D6"

    ---------

    Co-authored-by: thisiskeithb <13375512+thisiskeithb@users.noreply.github.com>
    Co-authored-by: Orel <37673727+0r31@users.noreply.github.com>

commit dca6afc
Author: Chris <52449218+shadow578@users.noreply.github.com>
Date:   Sun Apr 14 20:42:57 2024 +0200

    ✨🐛 HC32 - Add SERIAL_DMA, fix SDIO and MEATPACK (MarlinFirmware#26845)

    * fix meatpack on hc32

    * add support for SERIAL_DMA on HC32

    * add additional checks in HC32 HAL

    * migrate HC32 HAL to use app_config.h

    * fix memory leak in HC32 sdio HAL
    MarlinFirmware#26845 (comment)

    * hc32: fail if both EMERGENCY_PARSER and SERIAL_DMA are enabled

commit 19684f2
Author: Jason Smith <jason.inet@gmail.com>
Date:   Sat Apr 13 17:49:08 2024 -0700

    ✅ Add unit tests for macros.h (MarlinFirmware#26968)

commit 52a5613
Author: Keith Bennett <13375512+thisiskeithb@users.noreply.github.com>
Date:   Sat Apr 13 17:47:16 2024 -0700

    ⏪️ Revert unintended README changes (MarlinFirmware#26967)

    * Revert all the changes that went in with the unit test framework
    This restored broken links and other changes. Restoring to the prior revision seems the most appropriate action until the intentions of those file changes are known.
    ---------

    Co-authored-by: Jason Smith <jason.inet@gmail.com>

commit 0683e8a
Author: thinkyhead <thinkyhead@users.noreply.github.com>
Date:   Sun Apr 14 00:24:15 2024 +0000

    [cron] Bump distribution date (2024-04-14)

commit 1bb4a04
Author: Jason Smith <jason.inet@gmail.com>
Date:   Sat Apr 13 14:11:51 2024 -0700

    ✅Unit test improvements (MarlinFirmware#26965)

    * Do not warn about display in unit tests

    * Treat warnings as errors in unit tests

    * Report actual filenames with unit tests

commit d10861e
Author: Jason Smith <jason.inet@gmail.com>
Date:   Sat Apr 13 12:06:08 2024 -0700

    ✅ Add unit testing framework (MarlinFirmware#26948)

    - Add a framework to build and execute unit tests for Marlin.
    - Enable unit test execution as part of PR checks.

    ---------

    Co-authored-by: Costas Basdekis <costas.basdekis@gmail.com>
    Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>

commit cf7c86d
Author: Andrew <18502096+classicrocker883@users.noreply.github.com>
Date:   Sat Apr 13 14:59:59 2024 -0400

    🔧Fix M936 in features.ini (MarlinFirmware#26957)

commit d99e150
Author: David Buezas <dbuezas@users.noreply.github.com>
Date:   Sat Apr 13 18:54:25 2024 +0200

    ⚡️Reduce DISPLAY_SLEEP_MINUTES overhead (MarlinFirmware#26964)
@shadow578 shadow578 deleted the fix/usart-config-update branch May 12, 2024 08:40
RPGFabi pushed a commit to RPGFabi/Marlin that referenced this pull request Jun 15, 2024
* fix meatpack on hc32

* add support for SERIAL_DMA on HC32

* add additional checks in HC32 HAL

* migrate HC32 HAL to use app_config.h

* fix memory leak in HC32 sdio HAL
MarlinFirmware#26845 (comment)

* hc32: fail if both EMERGENCY_PARSER and SERIAL_DMA are enabled
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.

None yet

5 participants