-
Notifications
You must be signed in to change notification settings - Fork 2
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
Binary Serialization Integration #16
base: main
Are you sure you want to change the base?
Changes from all commits
69d6bfb
faab5cb
3c3757a
7651adc
5a26c11
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
# Authors | ||
|
||
- Gonzalo Casas <<[email protected]>> [@gonzalocasas](https://github.com/gonzalocasas) | ||
- Eleni Vasiliki Alexi <<[email protected]>> [@elenialex](https://github.com/elenialex) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,3 @@ | ||
compas>=1.17.6 | ||
paho-mqtt >=1, <2 | ||
msgpack |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,5 +1,6 @@ | ||||||
from compas.data import json_dumps | ||||||
from compas.data import json_loads | ||||||
import msgpack | ||||||
|
||||||
DEFAULT_TRANSPORT = None | ||||||
|
||||||
|
@@ -32,9 +33,10 @@ def set_default_transport(transport): | |||||
class Transport(object): | ||||||
"""Defines the base interface for different transport implementations.""" | ||||||
|
||||||
def __init__(self, *args, **kwargs): | ||||||
def __init__(self, codec=None, *args, **kwargs): | ||||||
super(Transport, self).__init__(*args, **kwargs) | ||||||
self._id_counter = 0 | ||||||
self.codec = codec or JsonMessageCodec() | ||||||
|
||||||
@property | ||||||
def id_counter(self): | ||||||
|
@@ -43,6 +45,7 @@ def id_counter(self): | |||||
return self._id_counter | ||||||
|
||||||
def publish(self, topic, message): | ||||||
self.codec.encode(message) | ||||||
pass | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This pass has no effect now, should be removed. But perhaps more importantly, this is probably not where we should be calling the encoding. This method is empty because the implementation is on the sub-classes of transport. So, I would leave it empty. |
||||||
|
||||||
def subscribe(self, topic, callback): | ||||||
|
@@ -58,6 +61,150 @@ def unadvertise(self, topic): | |||||
pass | ||||||
|
||||||
|
||||||
class MessageCodec: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In order to maintain IronPython compat, the class def should explicitly inherit from
Suggested change
Once we move fully to cpython, we can simplify this again. |
||||||
""" | ||||||
Base class for message codecs. | ||||||
|
||||||
This class defines the interface for message encoding and decoding. | ||||||
Subclasses should implement the `encode` and `decode` methods to handle | ||||||
specific serialization formats. | ||||||
""" | ||||||
|
||||||
def encode(self, message): | ||||||
""" | ||||||
Encode a message into a serialized format. | ||||||
|
||||||
Parameters | ||||||
---------- | ||||||
message : Message or dict | ||||||
The message to encode. | ||||||
|
||||||
Returns | ||||||
------- | ||||||
bytes or str | ||||||
The encoded message. | ||||||
|
||||||
Raises | ||||||
------ | ||||||
NotImplementedError | ||||||
If the method is not implemented by a subclass. | ||||||
""" | ||||||
raise NotImplementedError | ||||||
|
||||||
def decode(self, message): | ||||||
""" | ||||||
Decode a serialized message back into a `Message` object. | ||||||
|
||||||
Parameters | ||||||
---------- | ||||||
message : bytes or str | ||||||
The serialized message to decode. | ||||||
|
||||||
Returns | ||||||
------- | ||||||
Message | ||||||
The decoded message. | ||||||
|
||||||
Raises | ||||||
------ | ||||||
NotImplementedError | ||||||
If the method is not implemented by a subclass. | ||||||
""" | ||||||
raise NotImplementedError | ||||||
|
||||||
|
||||||
class JsonMessageCodec(MessageCodec): | ||||||
""" | ||||||
Message codec for JSON serialization. | ||||||
|
||||||
This codec handles encoding and decoding messages using JSON format. | ||||||
It supports messages that are instances of `Message` or dictionaries. | ||||||
""" | ||||||
|
||||||
def encode(self, message): | ||||||
""" | ||||||
Encode a message into a JSON string. | ||||||
|
||||||
Parameters | ||||||
---------- | ||||||
message : Message or dict | ||||||
The message to encode. | ||||||
|
||||||
Returns | ||||||
------- | ||||||
str | ||||||
The JSON-encoded message. | ||||||
""" | ||||||
if isinstance(message, Message): | ||||||
data = message.data | ||||||
else: | ||||||
data = message | ||||||
Comment on lines
+138
to
+141
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This logic of detecting the message type is a tad more complex (there's one more case to handle) and it should be handled centrally, so I would suggest we change
and this method can be much simpler:
... (cont'd) |
||||||
return json_dumps(data) | ||||||
|
||||||
def decode(self, message): | ||||||
""" | ||||||
Decode a JSON string back into a `Message` object. | ||||||
|
||||||
Parameters | ||||||
---------- | ||||||
message : bytes or str | ||||||
The JSON-encoded message to decode. | ||||||
|
||||||
Returns | ||||||
------- | ||||||
Message | ||||||
The decoded message. | ||||||
""" | ||||||
data = json_loads(message.decode("utf-8")) | ||||||
return Message(**data) | ||||||
|
||||||
|
||||||
class BinaryMessageCodec(MessageCodec): | ||||||
""" | ||||||
Message codec for binary serialization using MessagePack. | ||||||
|
||||||
This codec handles encoding and decoding messages using MessagePack. | ||||||
""" | ||||||
|
||||||
def encode(self, message): | ||||||
""" | ||||||
Encode a message into a binary format using MessagePack. | ||||||
|
||||||
Parameters | ||||||
---------- | ||||||
message : Message or dict | ||||||
The message to encode. | ||||||
|
||||||
Returns | ||||||
------- | ||||||
bytes | ||||||
The MessagePack-encoded message. | ||||||
""" | ||||||
|
||||||
if isinstance(message, Message): | ||||||
data = message.data | ||||||
else: | ||||||
data = message | ||||||
return msgpack.packb(data) | ||||||
Comment on lines
+184
to
+188
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ...and as mentioned in the other encode, this would also change to just do |
||||||
|
||||||
def decode(self, message): | ||||||
""" | ||||||
Decode a MessagePack binary message back into a `Message` object. | ||||||
|
||||||
Parameters | ||||||
---------- | ||||||
message : bytes | ||||||
The MessagePack-encoded message to decode. | ||||||
|
||||||
Returns | ||||||
------- | ||||||
Message | ||||||
The decoded message. | ||||||
""" | ||||||
data = msgpack.unpackb(message) | ||||||
return Message(**data) | ||||||
|
||||||
|
||||||
class Message(object): | ||||||
"""Message objects used for publishing and subscribing to/from topics. | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -60,8 +60,8 @@ def publish(self, topic, message): | |||||||||||||||||
""" | ||||||||||||||||||
|
||||||||||||||||||
def _callback(**kwargs): | ||||||||||||||||||
json_message = topic._message_to_json(message) | ||||||||||||||||||
self.client.publish(topic.name, json_message) | ||||||||||||||||||
encoded_message = self.codec.encode(message) | ||||||||||||||||||
self.client.publish(topic.name, encoded_message) | ||||||||||||||||||
Comment on lines
+63
to
+64
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Based on the comments above, this would change to call the conversion to data from the topic, and then use the codec for encoding:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the same would need to be done in |
||||||||||||||||||
|
||||||||||||||||||
self.on_ready(_callback) | ||||||||||||||||||
|
||||||||||||||||||
|
@@ -87,8 +87,11 @@ def subscribe(self, topic, callback): | |||||||||||||||||
subscribe_id = "{}:{}".format(event_key, id(callback)) | ||||||||||||||||||
|
||||||||||||||||||
def _local_callback(msg): | ||||||||||||||||||
msg = topic._message_from_json(msg.payload.decode()) | ||||||||||||||||||
callback(msg) | ||||||||||||||||||
decoded_message = self.codec.decode(msg.payload) | ||||||||||||||||||
callback(decoded_message) | ||||||||||||||||||
|
||||||||||||||||||
# msg = topic._message_from_json(msg.payload.decode()) | ||||||||||||||||||
# callback(msg) | ||||||||||||||||||
Comment on lines
+90
to
+94
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handling decoding needs to follow the same approach as the one commented on the encoding, because there's this custom type parsing supported in there, so, we need to change def _message_from_data(self, data):
"""Converts a decoded data object back into a message instance."""
return self.message_type.parse(data) and this needs to change a bit as well:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and the |
||||||||||||||||||
|
||||||||||||||||||
def _subscribe_callback(**kwargs): | ||||||||||||||||||
self.client.subscribe(topic.name) | ||||||||||||||||||
|
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
msgpack
is not available for ironpython, we should eithertry..except
this import or checkif compas.IPY
(and subsequently disable binary serialization if that's the case).