nmakel / solaredge_modbus

SolarEdge Modbus data collection library
MIT License
147 stars 36 forks source link

Modbus RTU 32-bit Floats Not decoded in correct order #43

Open greggcarpenteratpacketizedenergy opened 3 years ago

greggcarpenteratpacketizedenergy commented 3 years ago

I'm talking to a SolarEdge SE7600 on a Modbus RTU connection. When I read the CosPhi register using this library (register address 0xF002) which has a default value of 1.0 (0x3F800000) and which is reported in the RTU Modbus device reply packet as 0x010304 00003F80 EA63 the decode_32bit_float isn't correctly interpreting this as a 1.0 (probably due to inconsistencies in Modbus 32bit float conventions), but it's interpreting it 16-bitwise backwards as best I can tell. I also tried this for other registers (the handling for which haven't yet been implemented here) in the "Technical Note – Power Control Protocol for SolarEdge Inverters" document and the results are consistent in that it appears that the 32bit floats are decoded out of order (16-bitwise backwards).

nmakel commented 3 years ago

Thanks for reporting this. If you modify solaredge_modbus/__init__.py line 403 to read self.wordorder = Endian.Little, does that resolve the cosphi issue? Ignoring for a second that all the other values are then incorrect...

greggcarpenteratpacketizedenergy commented 3 years ago

Thanks kindly for the prompt reply!! I tested this and that fix does cause that specific read request to return the expected result, and doesn't appear to corrupt other reads I'm doing via this library (though since there aren't any other 32 bit reads with known values I'm not certain whether there are any other issues related to this). I haven't tried writing float values yet. I was also concerned that this issue might have been coming from an address misalignment (if the registers on both sides of the MSBs were both reading 0x0000 this could also look like that, but I'm not certain). Given that the prior register F001 1 R/W Active Power Limit Uint16 0-100 % behaves as expected I'm inclined to not think it's an address misalignment issue, however the float order I'm seeing would seem to be contrary to that reported in the Sunspec 32bit float specification: https://sunspec.org/wp-content/uploads/2015/06/SunSpec-Information-Models-12041.pdf

greggcarpenteratpacketizedenergy commented 3 years ago

Thanks kindly for the prompt reply!! I tested this and that fix does cause that specific read request to return the expected result, and doesn't appear to corrupt other reads I'm doing via this library (though since there aren't any other 32 bit reads with known values I'm not certain whether there are any other issues related to this). I haven't tried writing float values yet. I was also concerned that this issue might have been coming from an address misalignment (if the registers on both sides of the MSBs were both reading 0x0000 this could also look like that, but I'm not certain). Given that the prior register F001 1 R/W Active Power Limit Uint16 0-100 % behaves as expected I'm inclined to not think it's an address misalignment issue, however the float order I'm seeing would seem to be contrary to that reported in the Sunspec 32bit float specification: https://sunspec.org/wp-content/uploads/2015/06/SunSpec-Information-Models-12041.pdf

Update, I have written and reread the CosPhi register using this library mechanism after making the suggested Endian adjustment, and it appears to behave as intended. I'll update with any further developments I see with it.

nmakel commented 3 years ago

Thanks kindly for the prompt reply!! I tested this and that fix does cause that specific read request to return the expected result, and doesn't appear to corrupt other reads I'm doing via this library (though since there aren't any other 32 bit reads with known values I'm not certain whether there are any other issues related to this). I haven't tried writing float values yet. I was also concerned that this issue might have been coming from an address misalignment (if the registers on both sides of the MSBs were both reading 0x0000 this could also look like that, but I'm not certain). Given that the prior register F001 1 R/W Active Power Limit Uint16 0-100 % behaves as expected I'm inclined to not think it's an address misalignment issue, however the float order I'm seeing would seem to be contrary to that reported in the Sunspec 32bit float specification: https://sunspec.org/wp-content/uploads/2015/06/SunSpec-Information-Models-12041.pdf

Update, I have written and reread the CosPhi register using this library mechanism after making the suggested Endian adjustment, and it appears to behave as intended. I'll update with any further developments I see with it.

Thanks for the update. The other object which has float registers is the battery (starting at 0xe100). These values are read with a little endian wordorder. So it makes sense that the inverter model also uses little endian. But, as you say, there aren't that many 32bit float register to test with. Can you confirm the energy_total register still returns valid values?

Also, have you had any luck polling the other power control registers mentioned in the document you referred to?

greggcarpenteratpacketizedenergy commented 2 years ago

Can you confirm the energy_total register still returns valid values? On inspection it appears that this is NOT decoded correctly with the aforementioned wordorder set to Endian.Little, and decodes out of order as would be expected (packet reply 0x010304000000D27A6E which should be decoded as 210 is instead decoded as 13762560 as would be expected for having the wordorder set incorrectly).

However, regarding:

Also, have you had any luck polling the other power control registers mentioned in the document you referred to?

Those that my testing was aimed at primarily fall within the range 0xE000 - 0xE192 (as battery control is my main focus). Those DO appear to behave as desired across the range of registers tested. I've tested the following registers for both reading (and writing where applicable) with the wordorder = Endian.Little configuration:

0xE000 0xE004 0xE005 0xE006 0xE008 0xE00A 0xE00B 0xE00D 0xE00E 0xE010

And all seem to behave correctly (and have been exercised as such in my testing with the expected behavior registering at the inverter and measured power flow direction).