Various non-function affecting code cleanups #111
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The initial set up data for GS450H, GS300 and Prius are never modified and only used to send data to the inverter. They can be defined as const which saves RAM and time to copy them from Flash at startup. The dma_write function is updated to take a const pointer. This is safe as it is being transmitted via DMA and not modified as it would be for receive. Tests: - Compile test only
The data tables from JLR_G2 seem to have ended up in JLR_G1.cpp but are unused so remove them.
Various data tables in the JLR_G2 shifter are required to send CAN responses. They are only read from and are never modified. They can be defined as const which saves RAM and time to copy them from Flash at startup. Tests: - Compile test only
A lot of static functions and variables are defined in the LeafBMS class header but not implemented or used in the class or elsewhere in the project so can safely be removed. The exception is the LeafBMS::Temperature member which is assigned to during CAN decoding but never read from. Mark overridden virtual methods with "override" to reduce likelihood of future breakage should the parent method signature change. Tests: - Compile test only - Verify cppcheck reports no unexpected errors
Update the includes in kangoobms.h so that they are sufficient to define the KangooBMS class. Also update kangoobms.cpp to minimise the number of includes used. Mark overridden virtual methods with "override" to reduce likelihood of future breakage should the parent method signature change. Remove the GetCurrent() method from the interface as there is no straightforward way for the rest of the project to call this and it is otherwise unused. Tests: - Compile test only
No need to include all of stm32_vcu.h with dozens of headers. Just include the simpbms.h which has all of what the SimpBMS class implementation requires. Mark overridden virtual methods with "override" to reduce likelihood of future breakage should the parent method signature change Tests: - Build test
Fix the telsaCharger.h include guard typo. Improve the includes to use the minimal required to define the teslaCharger class. Fixup teslaCharger.cpp to have the few additional includes it requires. Mark overridden virtual methods with "override" to reduce likelihood of future breakage should the parent method signature change.
There is a collection of unused static members on the ISA shunt class from earlier versions of the code. They can safely be removed. Tests: - Code compile only
Remove the static designator from private methods in i3LIMClass. This gives the optimizer more opportunities to inline methods. The data should probably be moved to be class members but that is too intrusive at this time. Mark overridden virtual methods with "override" to reduce likelihood of future breakage should the parent method signature change. Tidy the includes.
Remove the unused members. Tidy the includes. Add "override" to the public methods.
Fix the daisychainbms.h headers to match what is required to define the class. Remove the catch all stm32_vcu.h from daisychainbms.cpp. Update the interface of DaisychainBMS to explicitly override parent methods.
The only place that needs to use the entire list of headers contained in stm32_vcu.h is stm32_vcu.cpp. Just paste the contents in. The CAN3_Msg union is no longer required so can be safely deleted. Tests: - Compile test with no errors
davefiddes
requested review from
damienmaguire and
jsphuebner
as code owners
|
Thanks Dave will review. Really appreciate the help:) |
damienmaguire
approved these changes
Sep 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Totally fine if you want to reject this but I was looking at the VCU code-base to see where things were at and me and my code linter (cppcheck) kept spotting minor problems. So I did a bit of tidying as I went.
There were various bits of non-obvious dead code highlighted. The linker probably removed a lot of this but it made it difficult to understand. I've left all of the commented out code as that looks to be WIP/documentation which is of value.
I found a few data tables which could be marked constant so they can stay in flash without causing any detriment.
There should be no function affecting changes which should break anything. I worked on the principle if compilation can't prove correctness I wasn't going to touch it.