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

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

Closed
1 task done
classicrocker883 opened this issue Jul 17, 2024 · 4 comments · Fixed by #27283
Closed
1 task done

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

classicrocker883 opened this issue Jul 17, 2024 · 4 comments · Fixed by #27283
Labels
Bug: Confirmed ! T: HAL & APIs Topic related to the HAL and internal APIs.

Comments

@classicrocker883
Copy link
Contributor

classicrocker883 commented Jul 17, 2024

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

  1. Load HC32 configs (provided)
  2. HC32F460C_aquila_101 in platformio.ini
  3. 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

  • 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
Copy link
Contributor

Hi @classicrocker883,

seems like the CRITICAL_SECTION_* macros are broken on the current bugfix branch.
i've created a PR at #27283, could you try it and report back if this fixes the issue?

@thisiskeithb thisiskeithb linked a pull request Jul 17, 2024 that will close this issue
@ellensp ellensp added Bug: Confirmed ! T: HAL & APIs Topic related to the HAL and internal APIs. and removed Bug: Potential ? labels Jul 17, 2024
@classicrocker883
Copy link
Contributor Author

@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
Copy link
Contributor

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

Copy link

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.

@github-actions github-actions bot locked and limited conversation to collaborators Sep 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug: Confirmed ! T: HAL & APIs Topic related to the HAL and internal APIs.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants