Skip to content
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

LLT/JBD BMS - Some changes for lost bluetooth connection / hci_uart stack restart #830

Merged
merged 2 commits into from
Dec 23, 2023

Conversation

mr-manuel
Copy link
Collaborator

@mr-manuel mr-manuel commented Oct 3, 2023

Fixes #777

except asyncio.TimeoutError:
logger.error(">>> ERROR: Unable to connect with BLE device")
return False
if self.hci_uart_ok:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here the else with a return false is missing

)
self.reset_bluetooth()
return False
if self.hci_uart_ok:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here also else with return false is missing

@mr-manuel mr-manuel marked this pull request as draft October 3, 2023 19:47
@mr-manuel
Copy link
Collaborator Author

@Marvo2011 just push a new commit on your dev branch and it will automatically appear here.

os.system("rmmod btbcm")
os.system("modprobe hci_uart")
os.system("modprobe btbcm")
sys.exit(1)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# start hciattach
/usr/bin/hciattach -t 10 /dev/ttyS0 bcm43xx 921600 noflow - &

#777 (comment) <- here you said that this line would be needed, but it's not part of the code changes. I was wondering if it is not needed after all?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't find where I said that. Is the link maybe wrong? :-)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The link leads to a comment by @Marvo2011 ;-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry now I get it :-) @Marvo2011 any progress with this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, I'm very busy at this weekend, I will take a look next week. For me it works but it's not perfect because it can take over a minute and in this time the system will shutdown the multi and other load outputs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@randomname32 a lot can go wrong, since on many BMS the SOC can be completely wrong, but the approach is good. I would use the cell voltage instead. If the min cell voltage is above 3.2 and the max cell voltage below 3.3. What do you think?

Copy link
Contributor

@Marvo2011 Marvo2011 Dec 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@randomname32 I read the needed hciattach start command from ps and hold it down to the file: /tmp/dbus-blebattery-hciattach Line 58 execute it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@randomname32 a lot can go wrong, since on many BMS the SOC can be completely wrong, but the approach is good. I would use the cell voltage instead. If the min cell voltage is above 3.2 and the max cell voltage below 3.3. What do you think?

Sounds good to me. But I am not an expert here, so maybe just add it as an optional thing...

Btw, today I found this option:

; --------- BMS disconnect behaviour ---------
; False: Charge and discharge is not blocked on BMS communication loss
BLOCK_ON_DISCONNECT = False 

Is this not taken into consideration for BLE connections?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@randomname32 yes, but only for 60 seconds. Then the driver disconnect. We are thinking to change that, if the cell voltages are between 3.2 and 3.3 volts.

def publish_battery(self, loop):
# This is called every battery.poll_interval milli second as set up per battery type to read and update the data
try:
# Call the battery's refresh_data function
result = self.battery.refresh_data()
if result:
# reset error variables
self.error["count"] = 0
self.battery.online = True
# unblock charge/discharge, if it was blocked when battery went offline
if utils.BLOCK_ON_DISCONNECT:
self.block_because_disconnect = False
else:
# update error variables
if self.error["count"] == 0:
self.error["timestamp_first"] = int(time())
self.error["timestamp_last"] = int(time())
self.error["count"] += 1
time_since_first_error = (
self.error["timestamp_last"] - self.error["timestamp_first"]
)
# if the battery did not update in 10 second, it's assumed to be offline
if time_since_first_error >= 10:
self.battery.online = False
self.battery.init_values()
# block charge/discharge
if utils.BLOCK_ON_DISCONNECT:
self.block_because_disconnect = True
# if the battery did not update in 60 second, it's assumed to be completely failed
if time_since_first_error >= 60:
loop.quit()
# This is to mannage CVCL
self.battery.manage_charge_voltage()
# This is to mannage CCL\DCL
self.battery.manage_charge_current()
# publish all the data from the battery object to dbus
self.publish_dbus()
except Exception:
traceback.print_exc()
loop.quit()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, you have pointed to that location before, but I did not fully read the code

@mr-manuel mr-manuel marked this pull request as ready for review December 23, 2023 09:22
@mr-manuel mr-manuel merged commit f67f363 into Louisvdw:dev Dec 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants