Create new methods for building packets.

[kipe]

As shown by issue #13, the current method for creating and sending packages is a bit complicated.
Maybe we should build methods for creating packages based on EEP.xml?

My initial idea is to create an Packet (class?) -method with the following style:

p = Packet.create(
    type=PACKET.RADIO,                     # required
    rorg=0xA5, func=0x02, type=0x06,       # required
    destination=[0x01, 0x03, 0x02, 0x13],  # optional, defaults to [0x00, 0x00, 0x00, 0x00] (broadcast)
    learn=False,                           # optional, default to false
    TMP=40.23                              # **kwargs, setting the values based on EEP.xml
)
c.send(p)

I can create the methods, but as I don't have any devices to send data to, at least help with testing would be required.

Comments?

[kipe]

Hmm, I wondered why this seemed a bit familiar 😆
Closed as invalid as per pull request #12

[sylvaincherrier]

Great !!
i will test... I have 2 different plug that i can use for this test !
I will grab them with me tomorrow, and make some tests.

Thanks a lot !

[kipe]

I'll try and write this suggested solution tonight, as it would make creation of packets even easier.

[sylvaincherrier]

Thx @kipe :-)

[sylvaincherrier]

Hi @kipe

I am testing the code.
I think it is

p = Packet.create( packet_type=PACKET.RADIO, # required ....

but then, i have 'enocean.protocol.eep: Cannot find type in EEP'

I am searching if rorg=0xA5, func=0x02, type=0x06 is correct.

[sylvaincherrier]

Oups...
Maybe it is the sender field that is the problem... Because nothing works, now...

[kipe]

Well, at least the EEP.xml in the project doesn't include a profile for 0XA5, 0x02, 0x06 ;)

[sylvaincherrier]

Ok :-)
I have to find which combinaison of rorg, func and type matches my plug, and then create lines in EEP.xml ? (if missing)

[kipe]

Basically yes. You should be able to get the information about the plug settings from the manufacturer, and the device ID should be printed on the label.
When creating the profile(s), please follow EnOcean Equipment Profiles 2.6.4. Also take note on how the current profiles are build and structured, so that they follow the documentation.

[sylvaincherrier]

Thanks !
My fist device has an EEP D2-01-01
The second one is EEP D2-02-0B
i make another try

[kipe]

Damn, I think VLD-messages are over all very much untested.
Also, the teach-in procedure seems quite tricky, as per page 223 in the EEP-documentation.

Not sure if I can even dare to try and write support for that without a device to test on :(

[sylvaincherrier]

I have an error : RORG is not supported by this function.
I think it is the same problem : Adding a description in EEP.xml ?

[kipe]

Unfortunately not... As mentioned in packet.py, the new function currently only support rorgs RPS, BS1 and BS4. Yours is VLD.
The issue with VLD is that it contains some stuff not taken into consideration in EEP.xml, such as message directions, variable message length etc.

This issue is already under consideration as issue #6. I can try and make a basic support based on the documentation at some point, but it does require a bit more thought 😕

[sylvaincherrier]

Ok...
If you need me to test different settings, just tell me. I can try to execute some codes.

[romor]

The sender and destination argument in Packet.create() is of type list. But the sender member of class RadioPacket is of type int. Thus, it is not possible to directly reply a packet with out_packet.destination = in_packet.sender.

Shouldn't we use type list for RadioPacket.sender RadioPacket.destination also?

[kipe]

Yes, yes, yes 👍
Very good point, I'll implement this at some point. Most probably it'll break a few things, such as my own implementations of the Communicator.

Reopening as a reminder.

//E: Maybe create an EnOceanID class for this, as it might help in the long run? Would get rid of creating the RadioPacket.sender_hex etc, which are only used for logging. You could just call str(RadioPacket.sender).

[romor]

One more comment with regard to class member types:
I think it would be more intuitive for users if Packet.data would correspond to the EnOcean data entries (i.e. DB0..DB3 for 4BS). I know that this would be a major change in the interface, so consider this just as some thoughts.

[kipe]

So you mean Packet.data should be basically a list of integers? Or a list of bitarrays?
One major issue I can see in this is that the data can span between multiple bytes, making it a lot harder to handle than the current version.

Can you post an example of usage, on how you'd like to use it?

[romor]

I think Packet.data could be a 4-byte list of integer for a BS4 packet. I think in general it is good to avoid redundant data. The RORG identifier is already stored in Packet.rorg and then again in Packet.data[0].

And a short example: For my HVAC device I need to teach-in with 4BS in Variation 3. This requires the EEP profile to be set also in the teach-in response packet. Currently I do this by response_packet.data[1:5] = request_packet.data[1:5]. I would consider it a bit nicer if I could just write response_packet.data = request_packet.data[:].

Packet.bit_data could remain unchanged but maybe act just as private data (Packet._bit_data)? Or do you think Packet._data should be private instead? If both is public than there is redundant data available again, which may get inconsistent (like it is in my teach-in response above).

[kipe]

Thank you for your comments.
I think using properties (@property) might be a good approach here, so Packet.data and Packet._bit_data would keep in sync. The main access method would be to set Packet.data, with possibility to change Packet._bit_data directly, if required.

I think I'll make an experiment to another branch when I have the time. Could you possibly create a test for the teach-in -procedure you're using, so I can test against it?

[kipe]

Some of the stuff discussed here is included in c2e3922 and 2022ae6.

As for the Packet.data containing stuff not corresponding to the EnOcean documentation, I haven't figured out a proper solution yet.
Might need to change the Packet.data to Packet.message and create a new Packet.data to reflect the actual, documented data? This would be a very major change, which I'm a bit reluctant to do. On the other hand, it would be fairly easy to do at this point and way more logical (for example, Packet.optional_data, Packet.sender etc already point to the correct data)...

[romor]

Might need to change the Packet.data to Packet.message and create a new Packet.data to reflect the actual, documented data?

From my perspective, this would be a good solution.

Packet could then only consist of Packet.message while data, rorg, status,... could all be properties just accessing the corresponding entry in Packet.message. Alternatively it could be the other way round such that Packet holds data, rorg, status, ... and Packet.message could just be generated from the other variables. The seconds looks more reasonable to me.

[kipe]

Damn, this really should be put on a priority, I once again managed to confuse myself with this issue :P

[kipe]

Closing, as most stuff is implemented. Moving the restructuring to #28.