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

Model 11 MAC address broken? #41

Open
andig opened this issue Jun 14, 2019 · 7 comments
Open

Model 11 MAC address broken? #41

andig opened this issue Jun 14, 2019 · 7 comments

Comments

@andig
Copy link

andig commented Jun 14, 2019

I'm trying to use gosunspec to work with an SMA grid inverter. Reading model 11 fails with a modbus exception when I try to read the first 6 words according to the model 11 definition:

<block len="13">
  <point id="Spd" offset="0" type="uint16" mandatory="true" units="Mbps" />
  <point id="CfgSt" offset="1" type="bitfield16" mandatory="true" >
    <symbol id="LINK">0</symbol>
    <symbol id="FULL_DUPLEX">1</symbol>
    <symbol id="AUTO_NEG1">2</symbol>
    <symbol id="AUTO_NEG2">3</symbol>
    <symbol id="AUTO_NEG3">4</symbol>
    <symbol id="RESET_REQUIRED">5</symbol>
    <symbol id="HW_FAULT">6</symbol>
  </point>
  <point id="St" offset="2" type="enum16" mandatory="true" >
    <symbol id="UNKNOWN">0</symbol>
    <symbol id="ENABLED">1</symbol>
    <symbol id="DISABLED">2</symbol>
    <symbol id="TESTING">3</symbol>
  </point>
  <point id="MAC" offset="3" type="eui48" />
  <point id="Nam" offset="7" type="string" len="4" access="rw" />
  <point id="Ctl" offset="11" type="bitfield16" access="rw" >
    <symbol id="AUTO">0</symbol>
    <symbol id="FULL_DUPLEX">1</symbol>
  </point>
  <point id="FrcSpd" offset="12" type="uint16" access="rw" units="Mbps" />
</block>

It does seem to work if I read 7 words. Is it possible that the MAC should actually start at offset 4 and a padding or else be applied before it?

@altendky
Copy link
Contributor

If we look at the official Python implementation of a SunSpec client it looks like an eui48 field is 64 bits/4 registers long with the lowest register address being 'unused'. Note the lack of use of data[0] and data[1] in data_to_eui48() and the prefixed b'\x00\x00' in eui48_to_data(). The spreadsheet at https://sunspec.org/wp-content/uploads/2017/09/SunSpecInformationModelReference20170928.xlsx agrees that the point length is 4.

https://github.com/sunspec/pysunspec/blob/5242ea9e4c687061cc0b8de4417b1943ed8f9264/sunspec/core/util.py#L75-L86

def data_to_eui48(data):
    if sys.version_info < (3,):
        data = [ord(x) for x in data]

    value = False
    for i in data:
        if i != 0:
            value = True
            break
    if value and len(data) == 8:
        return '%02X:%02X:%02X:%02X:%02X:%02X' % (
            data[2], data[3], data[4], data[5], data[6], data[7])

https://github.com/sunspec/pysunspec/blob/5242ea9e4c687061cc0b8de4417b1943ed8f9264/sunspec/core/util.py#L153-L154

def eui48_to_data(eui48):
    return (b'\x00\x00' + base64.b16decode(eui48.replace(':', '')))

But yeah, eui48 seems to be missing entirely from https://sunspec.org/wp-content/uploads/2015/06/SunSpec-Information-Models-12041.pdf (or my searching is failing).

Additionally, this model doesn't seem to follow alignment suggestions.

Use PAD to keep 32 and 64 bit alignment
As a consideration to low resource devices model designers should align all 32 and 64 bit values on even numbered offsets. A PAD register should be added at the end of the fixed length block and/or at the end of a repeating block to ensure both add up to an even number of registers. Arrange the other points as needed to place the PAD at the end of the block.

So either another 16-bit value should be moved before MAC and Nam or one that is before moved to after.

And finally, I wouldn't generally expect a Modbus exception when reading a group of registers which are a subset of another group you can read. Modbus shouldn't care if you are reading only the first three out of four registers that make up MAC. Unless maybe I've missed this as a deviation that SunSpec requires from normal Modbus.

@andig
Copy link
Author

andig commented Jun 14, 2019

Great, in-depth answer, thank you very much! As for

I wouldn't generally expect a Modbus exception when reading a group of registers which are a subset of another group you can read. Modbus shouldn't care if you are reading only the first three out of four registers that make up MAC.

I wouldn't know but can tell you that I've seen this behaviour in 5 different SMA devices, for whatever its worth...

@altendky
Copy link
Contributor

altendky commented Jun 14, 2019

Open items:

  • Documentation for eui48 type
  • Achieve even alignment for Nam and MAC?
    • Or will it be argued that these would be allocated on the inverter as an array of bytes and as such the alignment is not relevant?
  • Pad to even model length
  • Possibly add tests for both model length and multi-register point alignment

@cfstras
Copy link
Contributor

cfstras commented Jul 26, 2019

It seems like the "Not Implemented"-value for eui48 is also not defined. My first guess would have been 0x000.., since that's what string uses. However, pysunspec lists ff:ff:ff:ff:ff:ff (https://github.com/sunspec/pysunspec/blob/master/sunspec/core/suns.py#L80), which is underspecified -- are the unused bytes set to 0 or to ff?

@andig
Copy link
Author

andig commented Jul 26, 2019

I think it should only check starting at the second word as the first is ignored. Couln't immediately find it in the source though. Also see sunspec/pysunspec#74

@nielsbasjes
Copy link
Contributor

FYI: I have an SMA SunnyBoy 3.6 device that fills all 8 bytes with 0xFF if the MAC address is unspecified.

Can someone please post a 'real' value (i.e. all 8 bytes) when it is filled?
Main question: Do real devices put a 0XFF or a 0x00 in the first two bytes if it is filled?

@ghost
Copy link

ghost commented Aug 7, 2019

  1. In terms of documentation, yes eui48 type should be documented and that is in progress.
  2. For model 11 we cannot achieve alignment because the model is already being used by manufacturers.
  3. Type eui48 is implemented as uint64, so the unimplemented value is 0xFFFFFFFFFFFFFFFF
  4. As was described eui48 resides in the lower 6 bytes
  5. It seems that SMA requires that multi-register values be read together for synchronization purposes.

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

No branches or pull requests

4 participants