-
-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixing JK BMS black version. #24
Conversation
Adding JKBMS_CAN_CELL_COUNT to predefine number of cells. Fixing CAN reinstallation process.
Could you please make sure that your GitHub Actions are working and completing with success? You find more informations here: https://louisvdw.github.io/dbus-serialbattery/general/supported-bms#add-by-opening-a-pull-request |
; ----------Cell count ------------ | ||
; Predefine cell count for Jkbms_can | ||
; Should be autodetected if not set | ||
JKBMS_CAN_CELL_COUNT = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move this under ; --------- BMS specific settings ---------
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
https://github.com/p0l0us/dbus-serialbattery/actions/runs/8324561682/job/22776438759
But |
My previouse change related to Louisvdw#950 for JK bms can broke older "Black" JK bms devices. See fix in
jkbms_can.py
.JK BMS cells are updating very slowly via can bus and cell count can be increased few times before all detected. This causes exception in
dbushelpre.py
at lineself._dbusservice[cellpath % (str(i + 1))] = voltage
. In the end the cell voltage values were missing and not updated in VenusOS. So I added option to preset cell count for JK CANJKBMS_CAN_CELL_COUNT
which resolves the issue. Original logic is kept so the parameter is optional and not necessary if it works without.Last change in this PR fixes the reinstall shell script. It may happen that user has can2, but not can0 used by dbus-serialbattery. The original detection for can0 didn't work in such case. I included same fix for BLE even it may not be necessary.