Port pmic driver and add scheduler for it #1
Conversation
ocklin
commented
Feb 8, 2022
ocklin
commented
- move watchdog pmic driver to inverter lib
- make a template off it for easier test/mock
- break out specific platform dependent SPI class
- add a small scheduler class used to strobe watchdog
- add a small test program targeting inverter board
Ignoring parameters with ID==0 when loading
Added division support to CAN tx
Update the c2000 inverter application to use the gate driver. This involves turning on the gate drive PSU before initialising the gate drivers for each phase. The fault state of the gate drivers is reported to stdout every second. Add a simple heartbeat LED toggle to the main loop to compensate for the gate drive fault LED no longer being on (!) as a way of determinig if the inverter is powered up. Tests: - Run on the Tesla M3 inverter and verify it operates with the gate drive fault LED extinguished and not gate drive fault is reported.
Prior to merging this repo into the parent c2000-inverter repo we move the contents into a sub-directory with the same name. Follows the guide at https://medium.com/walkme-engineering/how-to-merge-a-git-submodule-into-its-main-repository-d83a215a319c
This moves the main STM32F1 inverter code into a platform directory so it follows the style and layout of the C2000 port. STM32 platform specific code remains in libopeninv. Tests: - Build all variants of the CMake build process - Build FOC and SINE variants with STM32 Makefile
There was a problem hiding this comment.
Back merging into a PR isn't good practice and makes for confusing git history. Best to leave merges to me.
Also, it seems out of scope of the PR which is about the basic PMIC driver and test application. I would prefer the integration into the inverter proper comes in a new PR. This should make things a lot more straight forward to understand, debug and test.
|
I have been testing this code. It mostly seems to work. The return result from As above, I'd really like to see the remaining issues in this code fixed and the PR merged. |
Arhh, indeed should not have ended up there. Let me see if I can revert that. |
|
Would you be able to rebase this PR to another branch? Only way I see to remove the back merge. |
|
|
Yep. I'll put this PR on a separate branch for PMIC integration work. I've had it done to my PRs but not done it myself. Bear with me while I read the Github docs on how best to do this. |
|
I've created a pmic-integration branch that starts where your first changeset started. Once you update the PR with your fixes I can merge this PR onto that branch. When you've got the inverter going I'll merge the branch back to "portable-cpp" (my master/main). |
Yes. I see the same in debugging. Not sure if its a bug or a feature though. printf() doesn't work for me. .text full. I would need to play with the ld scripts to accommodate. Probably due to too long path names in the debug info? Also on Mac the RTOS tools for printf seem broken.
Code issues addressed if I didn't miss any. |
|
Try this to fix the lack of space in .text. It adds another 8kB. To fix the Standby LDO perhaps we should just enable it even though it's a deviation from the Tesla config? Function should definitely not return anything other than Error::Ok in the test app. Shame about printf not working. The support in Linux/Windows isn't great compared to ARM semi-hosting but it does achieve working output. |
Looked through Tesla logs again, also started reading the datasheet in more depth. But didn't find anything new. Thus I can state only what we already know: original SPI traffic is different. It is never reading DEVSTAT. Instead it reads and then explicitly resets bits of IF, MONSFx and SYSSF, probably in response to an INT. I hope to get around to a bit of playing today. TGIF. |
|
That rings a bell. When I wrote the original code a year ago I decided to go for a super basic driver as Damien and Johannes were looking for it quickly (I had no HW then). The state change verification process was where I chose to deviate. That may have been a false economy. :( |
Normal engineering decision that we take 30 times per day. No worries, I will take that part. Still also need to verify if brute force reset of all registers is what is causing this and if it is even needed. I would suggest to disable DEVSTAT verification now, file a bug. Better to have the code in the main branch, it is not a regression. |
|
Agreed. Good to get your code landed. |
This change moves all of the STM32 specific parts of libopeninv into the platform/stm32f1 directory. This leaves the libopeninv directory in the top-level to contain only cross-platform code. No code changes were required only build system updates. Tests: - Build all CMake variants - Build FOC and SINE variants using the old stm32_sine Makefile
The C2000 inverter erroneously reported "Gate Drive PSU OFF" when it was in fact turned on. The PSU was in the desired state it was just the message. Tests: - Run on the Tesla M3 inverter and verify the updated message is displayed when running the inverter
|
No idea what happend - I merged your last updates to my portable-cpp branch. Now it looks like this PR points back to my portable-cpp instead of my pmicdriver-branch. |
|
The important thing to understand about github is that generally all of the merges on a project need to happen on github.com. Before a PR has been raised you can rebase or merge to your hearts content (I personally prefer rebase). Once a PR has been raised all you can/should do is push new commits. Merging is then something for the repo owner to deal with. If you'd pushed the remaining commits on your "pmicport" branch I would have happily (and speedily) merged this PR. |
