CommunityGD32Cores / ArduinoCore-GD32

Arduino core for GD32 devices, community developed, based on original GigaDevice's core
Other
89 stars 33 forks source link

SimpleFOC #95

Open Candas1 opened 1 year ago

Candas1 commented 1 year ago

Hi,

Just for your information, we are working on creating a gd32 driver for SimpleFOC. We are aiming for 6pwm using the advanced timer for now, and later work on the current sensing.

It's unfinished and untested so not ready to use.

Is there anything that can be useful to you? Like board definitions? Variants? New functions for advanced pwm?

maxgerhardt commented 1 year ago

I'd definitely be interested in custom boards being made so that they can be added here and to platform-gd32, so that they can be supported natively and with their right pin definitions.

I'd also like to know if the core gets "in the way" of implementing the firmware / library due to weird behavior, bugs or missing (expectable) functionality, so we can get a feedback loop back into the core.

Candas1 commented 1 year ago

Hi @maxgerhardt,

So far we got the drivers working well but using SPL directly. I want to use some of the arduino-gd32 functions to make the code more flexible. I have noticed a few things, maybe I can get your advice.

1) I get a pin number as a parameter and I want to get its timer and channel. I could be using this function that get's the information from PinMap_PWM, unfortunately if there are several alternative functions for a single pin, it always takes the first one. For example with PB15 I am using AF2 and I am expecting Timer0 and Channel 2 but it picks Timer14 Channel 1. [EDIT] I was wondering why setting those pins on STM32F103C8 is much more straight forward. It seems there are much less alternative functions but I notice they also use a different pin number for alternative functions like here [EDIT2] It looks like mbed-os went the extra mile for few GD32 chips, they also have the information if a pin is inverted, maybe this can be useful. [EDIT3] It looks like GD_PIN_FUNC_PWM is only 2 of the parameters, mbed-os use it all. But this means all the values should be used in PinMap_PWM, I am not sure if you guys automated this. [EDIT4] It would also help for this?

2) If using pinMode(PA10, OUTPUT); to initializer the pins for motor control. I does this pin_function(p, GD_PIN_FUNCTION3(PIN_MODE_OUTPUT, PIN_OTYPE_PP, 0)); But I would need this pin_function(p, GD_PIN_FUNCTION3(PIN_MODE_AF, PIN_OTYPE_PP, 2));

3) I am getting a deadtime in ns that I want to use when initializing the timer. Unfortunately the GD32 SPL doesn't seem to provide any helper function like this one from STM32. Would it be a problem if we copy and adapt this code and contribute back to arduino-gd32 ?

Sorry for sharing it all together here, I can create separate issues for better tracking.

Thanks in advance.

robcazzaro commented 1 year ago

Just a quick note that I have working code for point 3 above (timer deadtime calculation). I need to clean it up and test is across the entire range, but should provide a way to calculate the value to program in the register without using any ST copyrighted code. I'll share once tested, in case you want to add it to the GD32 code

Candas1 commented 1 year ago

Hi @maxgerhardt,

Have you had a chance to look at this?

Another enhancement that would be useful is to declare all the irq handlers as weak in case we want to define our own ones. If you agree I can create a PR for that.

Thanks in advance.

robcazzaro commented 1 year ago

@Candas1: please also see issue #106 for the weak definition of the IRQ handlers. I'm copying the relevant portion here:

"Just a quick note on the weak implementation: EXTI4_15_IRQHandler is already defined as weak here or here (I'm never sure which one is actually used)

I don't know if the linker complains if there are only 2 definitions (one is startup.s, one in gpio_interrupt.c), both weak. Once I add my definition, it will be ok for me, but the average user is unlikely to redefine it, so both definitions are weak and it's unclear to me which one the linker will choose. I'm sure you understand this all much better than I do, though 😉"

Candas1 commented 1 year ago

@robcazzaro I faced this issue a few days ago during my tests: image

And making this function weak worked.

robcazzaro commented 1 year ago

I'm not making myself clear, sorry. Let's try again.

There is already a weak definition in the startup_gd32f1x0.s assembler file. The GD32 library overwrites it with a non-weak definition in C, so it all works.

If you define the GD32 Arduino function in C as weak, then add yet another definition in your code, the linker sees two weak definitions (.s and .c), then one non-weak in your code, and it all works. That is what you did, and works just fine.

But if the library defines that function as weak in C, without the user adding their own definition, then the linker sees only 2 weak definitions in .s and .c, and doesn't know what to do, raising an error.

My point is that the GD32 Arduino library probably cannot define the IRQ handlers as weak because the average user, unlike you or me, will not redefine the IRQ handler in their code. I don't know how to solve this problem, but I'm flagging it for consideration.

Hope this makes more sense :)

Candas1 commented 1 year ago

ahhhhhh, ok 🤯

maxgerhardt commented 1 year ago

FYI I've not yet looked at it but hope to soon, along with doing a stable 1.0.0 release of this core.

Candas1 commented 1 year ago

Thanks Maximilian, please let us know if there is anything we can help with.