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

Update x1_mini_v34.py #89

Closed
wants to merge 1 commit into from
Closed

Conversation

jaykijay
Copy link

My V34 has a Data Length of 100 when using v3.003.02 Firmware

My V34 has a Data Length of 100 when using v3.003.02 Firmware
Copy link
Owner

@squishykid squishykid left a comment

Choose a reason for hiding this comment

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

Could you please add a test including a response from your inverter under the functional tests? Then we can make sure this keeps working

@jefft4
Copy link

jefft4 commented Mar 17, 2023

@squishykid - I need this fix applied and pushed through to the HA integration as my X1miniV34 has the same firmware as @jaykijay's. Can I do anything to help to move the PR along?

@kdehairy
Copy link
Contributor

I guess it's pending adding a test. As requested earlier here.

@jefft4
Copy link

jefft4 commented Mar 17, 2023

I guess it's pending adding a test. As requested earlier here.

I read that, but I'm not sure what that involves... if someone can tell me how to add a test, I have the same inverter so I can do that.

jefft4 added a commit to jefft4/solax that referenced this pull request Apr 27, 2023
Allow for 100 data length from x1 mini v3/4 inverters (jaykijay)
@jefft4
Copy link

jefft4 commented Apr 27, 2023

Could you please add a test including a response from your inverter under the functional tests? Then we can make sure this keeps working

@squishykid - looking at the files, tests/response.py and tests/expected_values.py for X1_MINI_V34 already match my inverter's results; there are 100 data points in the response definition and the expected values are all present and reasonable.

@Darsstar
Copy link
Contributor

Darsstar commented Apr 2, 2024

Can be closed, duplicates #139

@squishykid squishykid closed this Apr 2, 2024
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.

5 participants