Feature: LEAF charger + Cell monitoring #151
Conversation
Software/src/charger/nissanleaf.cpp
Outdated
| .MsgID = 0x59E, | ||
| .data = {0x00, 0x00, 0x0C, 0x76, 0x18, 0x00, 0x00, 0x00}}; | ||
|
|
||
| static uint8_t crctable[256] = { |
There was a problem hiding this comment.
Perhaps relocate out to a common header for leaf battery helper functions/variables/defines used by both the battery code and the charger?
Software/src/charger/nissanleaf.cpp
Outdated
| // 0x66 = 0.5A (2x) | ||
| // 0x65 = 0.3A (1x) | ||
| // 0x64 = no chg | ||
| // so 0x64=100. 0xA0=160. so 60 decimal steps. 1 step=100W??? |
There was a problem hiding this comment.
Not a suggestion but more thinking out loud. I see this comment block above echoed in Damien's PDM implementation too..to me, 60 100w steps is suspiciously close to the capacity of the 6.6kw charger. I do wonder about differences between the 3.3 and 6.6 versions
There was a problem hiding this comment.
Yes, I will clean this up once I get it confirmed working!
| String cellContent = "Cell " + String(i + 1) + "<br>" + String(cellvoltages[i]) + " mV"; | ||
|
|
||
| // Check if the cell voltage is below 3000, apply red color | ||
| if (cellvoltages[i] < 3000) { |
There was a problem hiding this comment.
wonder if we move this to a user setting with this as default... to account for differences between LFP and other cell types
There was a problem hiding this comment.
No more settings for users to mess up :) Later on we can make this smarter, and autodetect. Each battery file has defined min / max cell voltage ranges, so we should develop a way to use those. But since this PR is blocking other PRs, I suggest we merge this one as-is now as a start!
| extern uint16_t temperature_max; //C+1, Goes thru convert2unsignedint16 function (15.0C = 150, -15.0C = 65385) | ||
| extern uint16_t cell_max_voltage; //mV, 0-4350 | ||
| extern uint16_t cell_min_voltage; //mV, 0-4350 | ||
| extern uint16_t cellvoltages[120]; //mV 0-5000 per cell |
There was a problem hiding this comment.
also here, cell min voltage comment is 0-4350 and cell voltages array 0-5000... can be confusing
There was a problem hiding this comment.
Comments updated in 0befc4b
Software/src/charger/nissanleaf.cpp
Outdated
There was a problem hiding this comment.
It would be nice to keep with the kind of naming convention we have so far for the (physical) system components, i.e. NISSAN-LEAF-CHARGER.*
There was a problem hiding this comment.
Fixed in 6d7fe87 to match rest of repository standards
Software/src/charger/nissanleaf.h
Outdated
|
|
||
| extern uint16_t battery_voltage; //V+1, 0-500.0 (0-5000) | ||
|
|
||
| void update_values_can_nissanleaf_charger(); |
There was a problem hiding this comment.
Am I blind or is the definition missing for this function? If it's missing, I guess the declaration can be deleted. Still confused by the lack of definition since some check should complain if it doesn't exist (so maybe I AM blind) but the lack of function calls could mean that the declaration is ignored
There was a problem hiding this comment.
Yes it's unused, removed in 6d7fe87
Software/src/charger/nissanleaf.h
Outdated
| uint8_t calculate_CRC_Nissan(CAN_frame_t* frame); | ||
| uint8_t calculate_checksum_nibble(CAN_frame_t* frame); |
There was a problem hiding this comment.
These are (currently at least) used as local functions, so the declarations can be moved to .cpp and be made static
There was a problem hiding this comment.
Good catch, made static in 05b9e79
What
This PR adds the following features
Cell monitoring page in webui (All cellvoltages for LEAF + Tesla)
Test fake battery now has editable voltage in webui
LEAF PDM support, for emergency generator charging ⛽
Why
Popularly requested features
How
This is a mess of a PR, coded in a 7h marathon. It will have bugs in the PDM support, but I will squish those later when I test the generator charging at home.