search

stawel / cheali-charger

cheap lipo charger
824 stars 275 forks source link

Fix ARM Architectures #317

Closed joshgarde closed 2 years ago

joshgarde commented 2 years ago

Updates contained in this PR:

Further testing needs to be done on this PR as the new M051 BSP has not been confirmed to run the same as the CoIDE version

joshgarde commented 2 years ago

I just got a build running on my EV-Peak CQ3 (older models used a M0516LDN while newer ones use the NUC029LAN) with the new Nuvoton M051 BSP code. The charger code will be part of a different PR. Looks like this one is ready to merge. I'll wait a while to see if anyone has any comments/concerns.

stawel commented 2 years ago

Great pull request!

I do however have a question about: https://github.com/stawel/cheali-charger/pull/317/commits/710be6178c1ef19682fc4d8c64cbdf06706cbdb2 does it have to be such a big commit? maybe it could be divided into smaller commits? this would make reading the code much simpler, if it is ok with you, I will split it into ~5 commits tomorrow.

I'm asking because of this change (which I almost missed):

joshgarde commented 2 years ago

Yeah it should've probably been split up into more individual operations - renaming the folders, replacing the old CMSIS with the new one, then fixing the linker script. I kinda did it all in one go because I got caught up in fixing compiler errors and forgot to commit midway through

The crystal on my CQ3 with the M0516 is 12MHz so it might be good to define that based on which hardware is being built instead of hardcoding it into the system header

stawel commented 2 years ago

splitting the commit is more difficult than I thought :/ It will take a while longer :/ in the mean time I have a question about:

joshgarde commented 2 years ago

Closed with merge of https://github.com/stawel/cheali-charger/pull/319