Skip to content

Commit

Permalink
fix inverter type (#310)
Browse files Browse the repository at this point in the history
  • Loading branch information
rixxxx authored Jul 3, 2024
1 parent 256e5ef commit bc65f2a
Showing 1 changed file with 5 additions and 6 deletions.
11 changes: 5 additions & 6 deletions src/sunsynk/definitions/three_phase_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,12 +156,11 @@
0,
"Device type",
options={
0x0200: "Inverter",
0x0300: "Single phase hybrid",
0x0400: "Microinverter",
0x0500: "Low voltage three phase hybrid",
0x0600: "High voltage three phase hybrid 6-15kw",
0x0601: "High voltage three phase hybrid 20-50kw",
2: "Inverter",
3: "Single phase hybrid",
4: "Microinverter",
5: "Low voltage three phase hybrid",
6: "High voltage three phase hybrid",
},
),
FaultSensor((555, 556, 557, 558), "Fault"),
Expand Down

4 comments on commit bc65f2a

@bsafh
Copy link

@bsafh bsafh commented on bc65f2a Jul 3, 2024

Choose a reason for hiding this comment

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

after upgrade to the version with this fix, the addon gives errors (configuration unchanged)

14:20:15 ERROR NotImplementedError in read_wr_deye_2:
14:20:15 ERROR NotImplementedError in read_wr_deye_1:
14:25:00 ERROR (28 in 5 min) NotImplementedError in read_wr_deye_2:
14:25:00 ERROR (28 in 5 min) NotImplementedError in read_wr_deye_1:
14:30:00 ERROR (30 in 5 min) NotImplementedError in read_wr_deye_2:
14:30:00 ERROR (30 in 5 min) NotImplementedError in read_wr_deye_1:

@bsafh
Copy link

@bsafh bsafh commented on bc65f2a Jul 3, 2024

Choose a reason for hiding this comment

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

I think, lines 159 - 163 (the new ones) should be

            0x0200: "Inverter",
            0x0300: "Single phase hybrid",
            0x0400: "Microinverter",
            0x0500: "Low voltage three phase hybrid",
            0x0600: "High voltage three phase hybrid",

based on:

0X0200 组串机 inverter
0X0300 单相储能机 hybird
0X0400 微逆机 MI microinverter
0X0500 低压三相储能机 phase3 hybird
0X0600 高压三相储能机

and not just

2
3
4
5
6

@rixxxx
Copy link
Contributor Author

@rixxxx rixxxx commented on bc65f2a Jul 3, 2024

Choose a reason for hiding this comment

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

In your report
10:26:01 WARNING device_type: Unknown register value 6. Consider extending the definition with a PR. https://github.com/kellerza/sunsynk/tree/main/src/sunsynk/definitions

Your inverter has value 6 there, mine has 6 and I have collected 5 and 3 from the field. The documentation is clearly wrong.

The error you mention here is probably something else.

@bsafh
Copy link

@bsafh bsafh commented on bc65f2a Jul 3, 2024

Choose a reason for hiding this comment

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

Well, it's a code error, nothing I can do on the user side. The addon does throw a python error

exception NotImplementedError[¶](https://docs.python.org/3/library/exceptions.html#NotImplementedError)
This exception is derived from [RuntimeError](https://docs.python.org/3/library/exceptions.html#RuntimeError). In user defined base classes, abstract methods should raise this exception when they require derived classes to override the method, or while the class is being developed to indicate that the real implementation still needs to be added.

Note It should not be used to indicate that an operator or method is not meant to be supported at all – in that case either leave the operator / method undefined or, if a subclass, set it to [None](https://docs.python.org/3/library/constants.html#None).
Note NotImplementedError and [NotImplemented](https://docs.python.org/3/library/constants.html#NotImplemented) are not interchangeable, even though they have similar names and purposes. See NotImplemented for details on when to use it.

(from https://docs.python.org/3/library/exceptions.html )

I do not see

10:26:01 WARNING device_type: Unknown register value 6. Consider extending the definition with a PR. https://github.com/kellerza/sunsynk/tree/main/src/sunsynk/definitions 

anymore, probably because the NotImplementedError occurs before that WARNING can be issued.

In any case, the dev/edge addon does not work at the moment.

Please sign in to comment.