-
-
Notifications
You must be signed in to change notification settings - Fork 19.5k
Native GD32 Support for Creality V4.2.2 #27744
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
Conversation
…32F303RET6 mcu using MFL library
…repatation work before upstream pull request.
…e encrypt and it sets an STM32 specific define, which we can just ignore.
…r our u8glib changes are merged
Why are you not using standard pin name designation P{A-E}{0-15} as in every stm32 and the datasheet https://file.elecfans.com/web2/M00/2D/A4/pYYBAGHmFm-ABL9gABMJuXIE-eY432.pdf ? |
Yes, I am far too intimate with that Datasheet, as I wrote the low level library for this as well. The names are not meaningful in the context of the peripheral library due to the way the ports and pins are set in Hardware. It also simplified things and removed a lot of mapping back and forth to use pin numbers directly. That said, it is being considered for the Arduino Core. However, it would need to be worth the additional code and conversion overhead - and I am not set on it yet. If this is something you would rather wait to have before merging support in Marlin, then I am not sure what to say, since this is not a marlin requirement that I am aware of. Since the main purpose of this core (so far) is use in Marlin, your input is greatly valued. If this is something you really want, perhaps some macro definitions will work in the interim to allow their usage in the pins files? These would just be aliases though. If this sounds okay I could definitely add that here until something is adopted in the core. Edit: Since Arduino expects the direct pin number, within the function, where needed, the number is converted to a port and pin in an efficient manner, It seems inefficient to add additional mapping and remapping logic where it really is not required simply because one other core did it that way. So I am actually not entirely sure how this will be approached, but I think that is a little beyond the scope here, and is a discussion that could take place in the Arduino Core repo instead. Thanks so much!! Sidebar: |
This is why I wanted to use standard pins names. So you can use current pins files |
Thanks |
…and related overhead and use direct registers
4354891
to
efa1758
Compare
Overall, a very nice addition! In the long run it will be nice to get rid of our somewhat hacked-in GD32 support and replace it with a more specific implementation, so the integration of the MFL Library is really great, thanks!
There will be a big sprint following 2.1.3 to blast through some our dusty refactoring issues with an eye towards improving upgrading, extensibility, first-time setup, LCD adapters, user experience, etc. Whatever we can do to bring the code up to speed and modern C++ standards will be helpful to the overall modernization efforts, so please submit whatever will move us in that direction. If there are no other concerns about this PR then I will go ahead and merge it imminently. |
It does seem as though at least Creality has embraced GD32, so maybe the number of boards will increase at some point. Right now I believe it is just the 4.2.2, 4.2.7, and Aquila (4.2.2 pinout) and the newest cr4nxxxxx based boards. Would it be helpful to provide a base configuration.h for these boards? Or would it be preferred to have the user modify the Ender3-v2 one that is currently in the configuration repo? The only required changes are as noted in the PR, Serial is 0 instead of 1, and then the MB name change. |
One thing we can do is to make noise using a |
I found something which needs attentionwhat about however, the chip defined in firmware uses STM32F103RET6: #include "stm32f1/pins_CREALITY_CR4NS.h" // STM32F1 env:STM32F103RE_creality env:STM32F103RE_creality_maple so should this be switched to the new GD32F303 support? or both? |
After trying to build, I get these errors: Marlin/src/HAL/GD32_MFL/Servo.cpp: In function 'void fixServoTimerInterruptPriority()':
Marlin/src/HAL/GD32_MFL/Servo.cpp:44:20: error: 'getTimerUpIRQ' was not declared in this scope
44 | NVIC_SetPriority(getTimerUpIRQ(TIMER_SERVO), servo_interrupt_priority);
| ^~~~~~~~~~~~~
Marlin/src/HAL/GD32_MFL/Servo.cpp: In member function 'int8_t libServo::attach(int)':
Marlin/src/HAL/GD32_MFL/Servo.cpp:58:21: error: 'MAX_SERVOS' was not declared in this scope; did you mean 'HAS_SERVOS'?
58 | if (servoCount >= MAX_SERVOS) return -1;
| ^~~~~~~~~~
| HAS_SERVOS
Marlin/src/HAL/GD32_MFL/Servo.cpp: In member function 'int8_t libServo::attach(int, int, int)':
Marlin/src/HAL/GD32_MFL/Servo.cpp:67:21: error: 'MAX_SERVOS' was not declared in this scope; did you mean 'HAS_SERVOS'?
67 | if (servoCount >= MAX_SERVOS) return -1;
| ^~~~~~~~~~
| HAS_SERVOS
Marlin/src/HAL/GD32_MFL/Servo.cpp:59:16: error: 'servo_pin' was not declared in this scope; did you mean 'servoPin'?
59 | if (pin > 0) servo_pin = pin;
| ^~~~~~~~~
| servoPin
I may be able to solve one or two of these errors:
which leaves |
This board is not currently supported in the GD32 HAL or core. I can look into adding it though. If you are building for the supported boards, this is due to U8glib needing some defines for us. It was just merged, so once the release of that library gets updated in Marlin, it will build. |
See previous response. The include path just gets corrupted, but the updated U8glib HAL fixes these errors. Also, regarding support for other GD32 boards. I am in the process of acquiring them, which has been somewhat difficult as most suppliers ship the boards without knowing if the chip will be GD32 or STM32 (and in some cases HC32 or other). |
BOARD_CREALITY_V422_GD32_MFL with a GD32F303 This is very much meant to be a supported board. |
See: #27791 I personally use a MicroProbe V2 with the same pinout. It does not use the servo library. If the BLTouch does, this should fix it. Does the Aquila v1.0.1 come stock with the BLTouch? At any rate, it should now be supported. The initial support here is for the stock machines, since my ability to test upgrades is minimal. So, if someone with the hardware would test this, that would be wonderful. 🙂 |
Followup to MarlinFirmware#27744
@bmourit Hello sir. I am currently trying to port marlin for Easythreed K10 which is build on GD32F303RCT6. But I have no experience in this area, so I got no clue what to do beyond pins/config. I saw you did great job with GD32 here and for PlatformIO, maybe you could give me some advice how should I approach it? Any help is appreciated. |
Description
This provides native hardware Marlin HAL support for Creality V4.2.2 Boards using the GD32F303RET6 MCU.
It uses the MFL library and MFL Arduino Core that were developed in tandem with the Marlin HAL.
Requirements
Creality V4.2.2 board with GD32F303RET6 MCU.
Note:
Ender3 has been tested, however other machines with this board should all have the same pinout and therefor be supported. Other machines will need to be tested first and/or be marked as experimental. The same goes for custom hardware changes such as LCD, probes etc. Although I am using this now with MicroProve V2 without issue, LCD changes may require extra code to support. The default 128x64 LCD that comes with Ender3 is fully supported.
Benefits
Brings native GD32F303RET6 MCU support, which addresses the differences between STM32F103RET and this clone MCU.
Some of the main benefits are:
Binary code size and RAM usage are nearly identical to the STM32 HAL build, and in some cases may even be smaller, thanks to features in the latest ARM GCC package from PlatformIO
Configurations
Configurations will be very similar to the ones currently used, with the motherboard and serial port bing the two main things requiring change. STM32 Serial starts at 1, MFL GD32 starts at 0.
We are building with -std=gnu++23 which causes a number of deprecation warnings for Marlin. Ideally these would get their own pull request if desired, so for now we build with -Wno-deprecated-declarations to suppress the warnings.
Related Issues
Requires merge of #35