-
Notifications
You must be signed in to change notification settings - Fork 4
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
WIP: Add protocol schema handling #3
base: master
Are you sure you want to change the base?
Conversation
This is a schema definition that was generated by the code from the code in pilight/pilight#323. It is used until pilight merge it and give an option to get the actual schma of the running daemon. Signed-off-by: Jan Losinski <losinski@wh2.tu-dresden.de>
This implements a protocol schema registry that is filled by the pilight protocol schema definition. It provides and easy mechanism to validate protocol data and convert arguments to the correct datatypes. Signed-off-by: Jan Losinski <losinski@wh2.tu-dresden.de>
In this first version it uses only the static default protocol schema. As soon as pilight provides a mechanism to get this data from the daemon itself, we can replace this or make the source of the schema dependant from the daemon version. Signed-off-by: Jan Losinski <losinski@wh2.tu-dresden.de>
@DavidLP Even if the tests are failing and it is all still WIP I would appreciate to get some early feedback from you :) |
Signed-off-by: Jan Losinski <losinski@wh2.tu-dresden.de>
Signed-off-by: Jan Losinski <losinski@wh2.tu-dresden.de>
Signed-off-by: Jan Losinski <losinski@wh2.tu-dresden.de>
@@ -103,6 +105,11 @@ def __init__(self, host='127.0.0.1', port=5000, timeout=1, | |||
answer_1, answer_2) | |||
|
|||
self.callback = None | |||
self.protocol_registry = self._load_protocol_schema() | |||
|
|||
def _load_protocol_schema(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is a essential function to be used externally I would remove the _ prefix.
@@ -0,0 +1,2638 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is generated from development? We could incorporate a protocol version (e.g. release 7). But it is not that important.
self.schema) | ||
|
||
|
||
class ProtocolRegistry(object): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not get, why a class with validate is needed. This is what vol provides no?. We can just create a vol.Schema from the json dump.
Also can you add an example how to use the protocol validation schema?
I am generally fine with the changes, and have some minor comments. For sure we have to add unit tests and bump the coverage to 100% again ;-). Also I would like to know how to use this shown in an example. |
This tries to add protocol schema verification (basically checking/conversion of datatypes) for the protocol data sent to the daemon. In the first version it uses a static protocol schema dump generated from a patched pilight). The code to generate this is provided in [this PR](pilight/pilight#322.
As soon as this is merged we can detect the new feature from the pilight version and query it from the daemon itself.