Fix X1 Hybrid Gen4 support with new firmware

squishykid / solax

🌞 Solax Inverter API Wrapper

MIT License

100 stars 57 forks source link

Fix X1 Hybrid Gen4 support with new firmware #164

Open nazar-pc opened 3 months ago

nazar-pc commented 3 months ago

Fixes https://github.com/squishykid/solax/issues/161, new firmware just has a bunch more zeroes on this inverter. The whole approach with counting those is flawed though in my opinion judging by recent PRs. Maybe minimum is okay, but maximum clearly can change in newer firmwares.

MatthieuCoder commented 1 month ago

This is appending to me too

squishykid commented 2 weeks ago

Hi, thank you so much for raising a PR. Apologies for the late response.

Can you please add a test case which exercises this new behavior?

This recently merged PR can serve as an example for adding a new test fixture: https://github.com/squishykid/solax/pull/167/files#diff-5dbc88d6e5c3615caf10e32a9d6fc6ff683f5b5814948928cb84c3ab91c038b6

When you create an 'InverterUnderTest' in fixtures.py, just use the existing X1 Hybrid G4 inverter.

nazar-pc commented 2 weeks ago

This is a dead simple change, not sure it is actually required to have a whole test just for this. I'd argue checking the length of data is useless to begin with, if it wasn't present then this PR wouldn't be necessary. They just decided to add more zeroes in newer firmware and I suspect there will be more people coming over time with other inverters showing the same behavior with newer firmware on them.