olliiiver / sml_parser

Low memory C++ library to parse Smart Message Language (SML) data from smart meters.
GNU Lesser General Public License v2.1
34 stars 15 forks source link

Support for negative values #8

Closed deltaphi closed 1 year ago

deltaphi commented 1 year ago

In #6 I proposed a code change that would let the SML parser correctly handle negative values for two-byte values. In https://github.com/olliiiver/sml_parser/pull/6#issuecomment-1236415248, it was suggested that #7 should actually cover this issue. Unfortunately, quite the opposite is true: the code does now realiably not handle negative values for any length of data item. Instead of something like -230 I now get 65xxx - resulting in my code telling me that apparently my solar panel consumes 65kW instead of producing a couple 100 watts.

The change proposed in #6 relied on telling the compiler about the actual data type of the data being parsed and then leaving the conversion handling (in this case: Sign extension, possibly twos' complement converion) to the data type conversion of the compiler. The code from #7 removes all of that information and simply pastes the bit pattern received via SML to a long long int.

Switching on the length of the data item being received would be a rather simple approach to implement this. But seeing as this was just removed - what would be the most fitting approach to fix this? If it is only about removing duplicate code, I can try to whip up some template code that should reduce this - although that might not work on every compiler this code passes through.

deltaphi commented 1 year ago

See #9 for a proposed change.

deltaphi commented 1 year ago

See Code on Godbolt if you want to toy around with it.

olliiiver commented 1 year ago

Hi @deltaphi.

Thanks for coming back and spending time on this.

Let me understand how this works...

I haven't been aware that negative values are also a thing with energy meters, but its mentioned in the documentation:

https://www.bsi.bund.de/SharedDocs/Downloads/DE/BSI/Publikationen/TechnischeRichtlinien/TR03109/TR-03109-1_Anlage_Feinspezifikation_Drahtgebundene_LMN-Schnittstelle_Teilb.pdf?__blob=publicationFile&v=1 -> 6.2

So if the first bit from first byte is 1 , it's a negative number and all bits have to be inverted?

0xfefb would become 260 or 261?

int16_t a = 0xfefb;
a =~ a; // bitwise NOT 
std::cout << a << std::endl; // 260

Oliver

olliiiver commented 1 year ago

I think we must also check the Type-Length-Field -> signed vs unsigned value.

Can you provide a dump output from the OBIS value?

deltaphi commented 1 year ago

Hi @olliiiver,

this is bog-standard how to represent negative integer values. In a signed value, the highest order bit is the sign bit. If the highest order bit it 0, it's a positive value. If it is 1, it's a negative value. There is then multiple ways how the remainder of the value has to be interpreted: Is it an absoulte value and you just put the sign in front of it? In this case, 0xfefb would be -32507. Or do you invert all the other bits for a negative value? This is more convenient for computation - for details, I recommend reading up on Integer Representation on Wikipedia - the german Wikipedia Article on this might actually be easier to grasp.

From context, I can tell that my smart meter uses a complement representation. There is no way I am either providing or drawing 32kW - so it must be -260 or -261. I don't actually know whether my meter uses ones' complement or twos' complement, and at the current point in time it does not actually matter. I'm fine with the possible off-by-one and it does not actually affect the code in this PR. All the code does is to perform sign extension.

To elaborate on this a bit: say we use twos' complmenent, so 0xfefb (16bit value) represents -261. The bit 0x8000 is set, all the other bits have to be inverted (and added a decimal 1) to find the correct absolute value (261). If we want to cast the 16bit value to a 32bit value, what is the correct bit pattern to use? Intuitively, you might consider 0x0000fefb. However, as we look to the highest bit for the sign, the sign bit has now changed to 0, so this would be a positive number - can't be the solution. Another intuitive solution is to move the sign bit. This would give us 0x80007efb. However, if we apply the conversion rules, this gives us an absolute value of 0x7FFF8105 - and 2.147.451.141 Watts is even more off than before (at this point, maybe the wires in the walls start to glow or outright evaporate?). So the correct bit pattern is 0xfffffefb - if the sign is negative, you fill up with ones (and if it is positive, fill up with zeros).

deltaphi commented 1 year ago

I think we must also check the Type-Length-Field -> signed vs unsigned value.

Can you provide a dump output from the OBIS value?

I can look into providing a debug output from SML Parser tomorrow - is that what you are looking for?

However, I would really appreciate an override parameter. Smart Meters seem to have their own mind when it comes to what data they are sending. For example, the multiplier value for the total power on my meter is consistently off by three for my meter - i.e., in an SML message it claims to report kWh when it is actually reporting a Wh value.

olliiiver commented 1 year ago

Great. Yes, some hex dump (the serial debug output the lib provides) would be fine.

Is your type of meter available on https://github.com/devZer0/libsml-testing ? Would also like to implement a test for negative numbers ...

deltaphi commented 1 year ago

Hello @olliiiver ,

the Meter is an ISKRA MT631, which is not present in the list you linked to. You can find an example dump in this Gist. In this dump, the power value, 16.7.0, is -12.

deltaphi commented 1 year ago

Fixed by #9