Skip to content
This repository has been archived by the owner on Apr 22, 2024. It is now read-only.

Hello Messages compliance - optional elements #379

Closed
erickvermot opened this issue Jun 19, 2017 · 3 comments
Closed

Hello Messages compliance - optional elements #379

erickvermot opened this issue Jun 19, 2017 · 3 comments

Comments

@erickvermot
Copy link
Contributor

Hello messages should be able to hold a body with optional elements, but pyof v0x01 currently does not accept hello messages with a body.

Any element should be accepted, for there may be expansion in the future.
For now, only VERSIONBITMAP elements are known, and hello messages should be able to carry it.

This is the same problem as stated in as #303, but was only developed for v0x04.
The proposed code is not working though, and need fix. (#378)
I suggest we fix it and then simply copy the code to the v0x01 lib.

@diraol
Copy link
Contributor

diraol commented Jun 20, 2017

@erickvermot considering #157 (class improved inheritance), I think that an easier solution would be to implement a GenericHeader class in foundation.common that implements a generic header, independent from any specific version.
The all specific version headers would inherit from this GenericHeader and change the necessary values (such as the version attribute value).
Then, if one need to use the unpack of a generic version, on can use this GenericHeader class.

This is just an example on how to deal with this issue (and all the associated issues) considering the inheritance improvements I've made (but are not yet merged on this repo).

@diraol diraol added this to the 2017.2 milestone Jun 20, 2017
@erickvermot
Copy link
Contributor Author

@diraol I see your point with the genericheader, and I think it is good for the unpack of a generic version as you said. But I think you misunderstood the issue for in this issue I am referring to the specific v0x01 Hello message.
It simply should accept a body, and optional elements, including the versionbitmap, which it currently does not.

@cemsbr
Copy link
Contributor

cemsbr commented Aug 3, 2017

From the spec: "Implementations must be prepared to receive a hello message that includes a body, ignoring its contents, to allow for later extensions". As nobody is using it yet, I'm lowering the priority.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants