search

dreadnought / python-daly-bms

Python module for Daly BMS devices
MIT License
82 stars 37 forks source link

3S BMS single data frame in get_cell_voltages #5

Closed n-benchoubane closed 2 years ago

n-benchoubane commented 2 years ago

Hello !

I did the experiment on the 3S BMS and I got the following error with the get_cell_voltagesfunction:

TypeError: a bytes-like object is required, not 'int' (Line https://github.com/dreadnought/python-daly-bms/blob/cddafd09dbe1115f085d006ff46ad9ee21b97794/dalybms/daly_bms.py#L257)

response_data = b'\x01\x0e`\x0e\x9a\x00'

After looking at the get_temperaturesfunction, I noticed that the response_datawas a list instead of an array of bytes.

Thus, in BMS 3S, we only get one frame of data for the cell voltages. I have included the following fix:

    def get_cell_voltages(self, response_data=None):
        if not response_data:
            max_responses = self._calc_cell_voltage_responses()
            if not max_responses:
                return
            response_data = self._read_request("95", max_responses=max_responses)
        if not response_data:
            return False

        if not isinstance(response_data, list):
            response_data = [response_data, bytes([0] * len(response_data))]

        cell_voltages = self._split_frames(response_data=response_data, status_field="cells", structure=">b 3h x")

        for id in cell_voltages:
            cell_voltages[id] = cell_voltages[id] / 1000
        return cell_voltages

Is there another way around it?

Many thanks to you!

dreadnought commented 2 years ago

Please test my latest commit: https://github.com/dreadnought/python-daly-bms/commit/f3928a39586da72190ff6a41e2af6560464e4567

get_cell_voltages was handling it better than get_temperatures actually, but _read function had a bug. get_temperatures had max_responses=3 hard-coded, so it was always sending 3 requests and returned a list, even when there was only one sensor. Now it's the same in both functions and the bug should be fixed.

n-benchoubane commented 2 years ago

Thank you very much! It works perfectly.

I was curious if you don't mind me asking about how to get the n_cycles for the sinowealth if it is possible.

dreadnought commented 2 years ago

You're welcome. It seems to be possible, as I see the cycles field in the Windows app. I've added it to --status now (https://github.com/dreadnought/python-daly-bms/commit/751b395834bf2f710eb0bef1b368263920dc163e), you can test it and report back if the returned value looks realistic.

n-benchoubane commented 2 years ago

Brilliant! It works really well :) Thank you very much.