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

Sinowealth based Daly BMS support #41

Merged
merged 3 commits into from
Sep 6, 2021

Conversation

SanderV4n
Copy link
Contributor

@SanderV4n SanderV4n commented Sep 4, 2021

This implements support for Sinowealth chipset based BMS, for now limited to SH367303 / 367305 / 367306 / 39F003 / 39F004 / BMS_10 chips with up to 10 cells. I have tested this with my Daly SmartBMS 4S 250A BMS, this is based on the SH39F003 chipset. My battery bank is a lishen 4s3p, 12 cell 840 Ah 12V bank installed on my boat. Using a solar MPPT and Multiplus 12/3000 for charging and Venus GX device + BMV712 for controlling the setup.

What works:

  • Reading SOC, Voltage and Current
  • Reading Cell Voltages and passing Min and Max values
  • Reading capacity and capacity remaining
  • Reading BMS status, charge/discharge fets (not all cases tested)
  • Reading Alarm flags (not all cases tested)
  • Reading temperature

What kinda works

  • Reading cycles
    Reading cycles is implemented, but does not match bluetooth app value

What is not implemented
The BMS has more data avaiable in the app, I am not able to read this data atm. It seems there is some way to read "extended parameter" pages, no clue on how to do this. Because of this some values are hard coded, like cell voltage thresholds and discharge and charge current limits.

On this pull request

  • Python is not my first language, so codestyle and code improvemt suggestions are welcome.
  • It is only tested on my setup, in limited fasion, let me know what works and does not work in your case
  • My vacation ends tomorrow, so I have less/no access to the setup on a daily basis and have to see when I am back at my boat to test new changes.
  • I am not sure on what needs to be done to complete this pullrequest, so @Louisvdw please fill me in on what is missing.

@Louisvdw Louisvdw self-assigned this Sep 6, 2021
@Louisvdw Louisvdw linked an issue Sep 6, 2021 that may be closed by this pull request
etc/dbus-serialbattery/sinowealth.py Outdated Show resolved Hide resolved
etc/dbus-serialbattery/utils.py Outdated Show resolved Hide resolved
etc/dbus-serialbattery/sinowealth.py Outdated Show resolved Hide resolved
etc/dbus-serialbattery/sinowealth.py Outdated Show resolved Hide resolved
etc/dbus-serialbattery/sinowealth.py Show resolved Hide resolved
@SanderV4n
Copy link
Contributor Author

SanderV4n commented Sep 6, 2021

@Louisvdw thanks for the review, valid points! I will make the changes and try to test them before posting again.

@Louisvdw
Copy link
Owner

Louisvdw commented Sep 6, 2021

@Carstijn
Here is an example of how your read_cell_data() function can be changed to use the Cell object (for some reason I get a github error page trying to create a pull request on your file/repo.

def read_cell_data(self):
	if self.cell_count is None:
	  self.read_pack_config_data()
	# clear the list
	for c in self.cells:
		self.cells.remove(c)
	# get up to the first 16 cells
	for c in range(self.cell_count):
		cell = Cell(False)
		cell.voltage = self.read_cell_voltage(c+1)
		if self.cell_voltages[cell_index] is False:
			return False
		self.cells.append(cell)
	return True

@SanderV4n
Copy link
Contributor Author

@Louisvdw I made some changes, to fix your comments, pleas re-review.

About the list, creating and deleting it does not feel very optimal, do you have something about my latest solution? (creating once in settings and only updating)

Copy link
Owner

@Louisvdw Louisvdw left a comment

Choose a reason for hiding this comment

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

Great work adding the Sinowealth Daly

@Louisvdw Louisvdw merged commit 404d894 into Louisvdw:master Sep 6, 2021
@SanderV4n SanderV4n deleted the feature/DalySinowealth branch February 13, 2023 17:46
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.

Daly 3S and 4S BMS Support
2 participants