[BUG] HC32 + ProUI `__iCliRetVal / __iSeiRetVal not declared in this scope`

classicrocker883 commented 3 months ago

Did you test the latest `bugfix-2.1.x` code?

Yes, and the problem still exists.

Bug Description

paging @shadow578

These two functions not found:

HAL/HC32/HAL.h:122:9: error: '__iCliRetVal' was not declared in this scope
HAL/HC32/HAL.h:126:9: error: '__iSeiRetVal' was not declared in this scope

Marlin\src\HAL\HC32\HAL.h

#define CRITICAL_SECTION_START        \
  uint32_t primask = __get_PRIMASK(); \
  (void)__iCliRetVal()

#define CRITICAL_SECTION_END \
  if (!primask)              \
  (void)__iSeiRetVal()

so apparently __iSeiRetVal and __iCliRetVal are found in .platformio\packages\toolchain-atmelavr\avr\include\util\atomic.h

I noticed in Marlin\src\HAL\STM32F1\HAL.h it has #include <util/atomic.h> which HC32 does not.

The issue resides here, in libs\buzzer.cpp, in function Buzzer::tick():

      #if ENABLED(EXTENSIBLE_UI) && DISABLED(EXTUI_LOCAL_BEEPER)
        CRITICAL_SECTION_START();
        ExtUI::onPlayTone(state.tone.frequency, state.tone.duration);
        CRITICAL_SECTION_END();

Since ProUI is now an EXTENSIBLE_UI, this get's enabled as well, otherwise it would not be an issue.

A workaround is to simply omit it if defined(ARDUINO_ARCH_HC32): e.g.:

#if ENABLED(EXTENSIBLE_UI) && NONE(EXTUI_LOCAL_BEEPER, ARDUINO_ARCH_HC32)

ProUI doesn't have a useable onPlayTone()
Otherwise somehow make the HC32/HAL.h utilize the atomic.h file.

or lastly remove ProUI from EXTENSIBLE_UI

Bug Timeline

just noticed, but probably when ProUI became under the EXTENSIBLE_UI umbrella

Expected behavior

No response

Actual behavior

Error when compiling

Steps to Reproduce

Load HC32 configs (provided)
HC32F460C_aquila_101 in platformio.ini
Compile

Version of Marlin Firmware

bugfix-2.1.x

Printer model

No response

Electronics

No response

LCD/Controller

No response

Other add-ons

No response

Bed Leveling

None

Your Slicer

None

Host Software

None

Don't forget to include

[X] A ZIP file containing your Configuration.h and Configuration_adv.h.

Additional information & file uploads

HC32configs.zip

[!NOTE] in Configuration.h you may need to edit #define HEATER_0_MAXTEMP 315 to be 275

shadow578 commented 3 months ago

Hi @classicrocker883,

seems like the CRITICAL_SECTION_* macros are broken on the current bugfix branch. i've created a PR at https://github.com/MarlinFirmware/Marlin/pull/27283, could you try it and report back if this fixes the issue?

classicrocker883 commented 3 months ago

@shadow578 yes it compiles correctly.

my only concern was your fix copies HAL/STM32/HAL.h and not HAL/STM32F1/HAL.h. basically, I thought HC32 is more like STM32F1 (Maple) versus STM32, but CRITICAL_SECTION_ doesn't play any pivotal role regardless.

so I know the HC32 is basically a clone of STM32F103, which one is it more like? the STM32, or STM32F1 (Maple)?

shadow578 commented 3 months ago

so I know the HC32 is basically a clone of STM32F103,

not really, while some peripherals work similarily, both chips are fairly different still. For one, the STM32F103 has a Cortex M3 core, while the HC32F460 has a M4 core

which one is it more like? the STM32, or STM32F1 (Maple)?

Voxelab based their firmware's hal on the STM32F1. But since then, I've basically rewritten the full thing, referencing the STM32 hal & Arduino core for STM32, SAMD21 and nRF5.

The version of the HC32 hal currently in marlin is loosely similar to the STM32 hal, but not in a way that normally allows copying code.

In this case, the implement in STM32 and HC32 hal are similar since both directly use functions from CMSIS

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

MarlinFirmware / Marlin