Addition of PDK Shifter & Initial MLBEVO Charger #180
Conversation
This file contains hidden or 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
…lso separated MagicByte Calc to a separate function for global use.
Mitchy6
requested review from
damienmaguire,
jsphuebner and
Tom-evnut
as code owners
|
Has this been bench tested and verified working? |
crasbe
reviewed
Jul 8, 2025
There was a problem hiding this comment.
I went through the code and have some annotations about the code style.
One thing that is really important IMO would be more documentation for the functions. Many of the functions do a lot and it would be good to document on a higher level what they do.
The vag_utils files look oddly familiar and I'm not really sure if the Copyright notice is right 👀
Comment on lines
+47
to
+48
| OutlanderHeartBeat.o E65_Lever.o leafbms.o V_Classic.o kangoobms.o OutlanderCanHeater.o VWAirHeater.o\ | ||
| MGCoolantHeater.o NissLeafMng.o preheater.o MGgen2V2Lcharger.o Gear_9x1.o vw_mlb_charger.o vag_utils.o\ |
There was a problem hiding this comment.
Suggested change
| OutlanderHeartBeat.o E65_Lever.o leafbms.o V_Classic.o kangoobms.o OutlanderCanHeater.o VWAirHeater.o\ | |
| MGCoolantHeater.o NissLeafMng.o preheater.o MGgen2V2Lcharger.o Gear_9x1.o vw_mlb_charger.o vag_utils.o\ | |
| OutlanderHeartBeat.o E65_Lever.o leafbms.o V_Classic.o kangoobms.o OutlanderCanHeater.o VWAirHeater.o \ | |
| MGCoolantHeater.o NissLeafMng.o preheater.o MGgen2V2Lcharger.o Gear_9x1.o vw_mlb_charger.o vag_utils.o \ |
Comment on lines
+20
to
+21
| #ifndef Gear_9x1_h | ||
| #define Gear_9x1_h |
There was a problem hiding this comment.
Suggested change
| #ifndef Gear_9x1_h | |
| #define Gear_9x1_h | |
| #ifndef GEAR_9X1_H | |
| #define GEAR_9X1_H |
Typically you write the include guard in all upper-case (most defines for that matter).
Comment on lines
+40
to
+42
| #endif // Gear_9x1_INCLUDED | ||
|
|
||
|
|
There was a problem hiding this comment.
Suggested change
| #endif // Gear_9x1_INCLUDED | |
| #endif // GEAR_9X1_H |
| Elcon = 6, | ||
| MGgen2=7 | ||
| MGgen2 = 7, | ||
| MLBevo = 8 |
There was a problem hiding this comment.
Suggested change
| MLBevo = 8 | |
| MLBevo = 8, |
It's good practise to give the last entry a comma as well, so you don't have to change it when adding an entry.
Comment on lines
+437
to
+438
| BMWE65 =4, | ||
| PorschePDK =5 |
There was a problem hiding this comment.
Suggested change
| BMWE65 =4, | |
| PorschePDK =5 | |
| BMWE65 = 4, | |
| PorschePDK = 5, |
Comment on lines
+231
to
+250
| void VWMLBClass::Task1Ms() | ||
| { static uint8_t counter1ms = 0; | ||
| counter1ms++; // Increments every 10ms | ||
| if(vehicle_status.CANQuiet == 1){ counter1ms = 0; } // Quick way of silencing the canbus -- NOT IMPLEMENTED | ||
| // - Do 10ms Tasks: | ||
| if (counter1ms % 10 == 0) { // Every 100ms (10ms * 1) | ||
| msg191(); // BMS_01 0x191 CRC | ||
| } | ||
| // - Do 20ms, 40ms, 50ms tasks: | ||
| if (counter1ms % 20 == 0) { // Every 20ms (10ms * 2) | ||
| } | ||
| if (counter1ms % 40 == 0) { // Every 40ms (10ms * 4) | ||
| //msg040(); // Airbag_01 - 0x40 CRC | ||
| can->Send(0x2B1, MSG_TME_02, 8); // MSG_TME_02 0x2B1 | ||
| } | ||
| if (counter1ms % 50 == 0) { // Every 50ms (10ms * 5) | ||
| msg2AE(); // DCDC_01 0x2AE CRC | ||
| counter1ms = 1; | ||
| } | ||
| } |
There was a problem hiding this comment.
The indentation in this function has to be fixed too.
| } | ||
|
|
||
|
|
||
| void Gear_9x1::DecodeCAN(int id, uint32_t* data) |
There was a problem hiding this comment.
This function has an indentation level of 2.
Comment on lines
+692
to
+700
| //BMS charge current limit but needs to be power for most AC charger types. | ||
| if(charger_params.HVcur > 1000) | ||
| { | ||
| charger_params.calcpwr = 12000; | ||
| } | ||
| else | ||
| { | ||
| charger_params.calcpwr = charger_params.HVcur*(battery_status.BMSVoltx10/10); | ||
| } |
There was a problem hiding this comment.
Suggested change
| //BMS charge current limit but needs to be power for most AC charger types. | |
| if(charger_params.HVcur > 1000) | |
| { | |
| charger_params.calcpwr = 12000; | |
| } | |
| else | |
| { | |
| charger_params.calcpwr = charger_params.HVcur*(battery_status.BMSVoltx10/10); | |
| } | |
| //BMS charge current limit but needs to be power for most AC charger types. | |
| if(charger_params.HVcur > 1000) | |
| { | |
| charger_params.calcpwr = 12000; | |
| } | |
| else | |
| { | |
| charger_params.calcpwr = charger_params.HVcur*(battery_status.BMSVoltx10 / 10); | |
| } |
| // ESP15 | ||
| HMS_Systemstatus = 3 ; //0 "No_function_active" 1 "Hold_active" 2 "Parking_requested" 3 "Parking_active" 4 "Keep parking_active" 5 "Start_active" 6 "Release_request_active" 7 "Release_request_by_driver" 8 "Slipping_detected" 9 "Hold_standby_active" 10 "Start_standby_active" 14 "Init" 15 "Error " ; | ||
| HMS_aktives_System = 6; //0 "No_System__Init_Error" 1 "Driver request_active" 2 "HMS_internal_active" 3 "ACC_active" 4 "Autohold_active" 5 "HHC_active" 6 "HVLM_active" 7 "Getriebe_aktiv" 8 "EBKV_aktiv" 9 "ParkAssist_aktiv" 10 "ARA_aktiv" 12 "Autonomous_Hold_aktiv" 13 "STA_aktiv " 14 "Motor_aktiv" 15 "EA_aktiv" 16 "VLK_aktiv" ; | ||
|
|
There was a problem hiding this comment.
From here the indentation changes from 4 to 2.
| void Task10Ms(); | ||
| void Task100Ms(); | ||
| void DecodeCAN(int, uint32_t*); | ||
| bool GetGear(Shifter::Sgear& outGear);//if shifter class knows gear return true and set dir |
There was a problem hiding this comment.
Suggested change
| bool GetGear(Shifter::Sgear& outGear);//if shifter class knows gear return true and set dir | |
| bool GetGear(Shifter::Sgear& outGear); // if shifter class knows gear return true and set dir |
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.
Addition of PDK Shifter & Initial MLBEVO Charger via CAN1 or CAN2 - Also separated MagicByte Calc to a separate function for global use.
Magic-byte table can be trimmed back slightly if space is a concern.