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

opack: implement dates #2466

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions pyatv/support/opack.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
"""Support for the OPACK serialization format.

Notes:
* Absolute time (0x06) is not implemented (can unpack as integer, not pack)
* Pack implementation does not implement UID referencing
* Likely other cases missing
"""
Expand All @@ -15,6 +14,8 @@

_SIZED_INT_TYPES: Dict[int, Type] = {}

_OPACK_TIME_OFFSET: float = 978307200.0
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment (and possibly rename the constant) making it clear that this is a conversion from offset between unix time and mach absolute time? This is a very magic constant.



def _sized_int(value: int, size: int) -> int:
"""Return an int subclass with a size attribute.
Expand Down Expand Up @@ -44,7 +45,9 @@ def _pack(data, object_list):
elif isinstance(data, UUID):
packed_bytes = b"\x05" + data.bytes
elif isinstance(data, datetime):
raise NotImplementedError("absolute time")
packed_bytes = b"\x06" + struct.pack(
">d", data.timestamp() - _OPACK_TIME_OFFSET
)
elif isinstance(data, int):
size_hint = getattr(data, "size", None) # if created with _sized_int()
if data < 0x28 and not size_hint:
Expand Down Expand Up @@ -154,8 +157,12 @@ def _unpack(data, object_list):
value = UUID(bytes=data[1:17])
remaining = data[17:]
elif data[0] == 0x06:
# TODO: Dummy implementation: only parse as integer
value, remaining = int.from_bytes(data[1:9], byteorder="little"), data[9:]
value, remaining = (
datetime.fromtimestamp(
struct.unpack(">d", data[1:9])[0] + _OPACK_TIME_OFFSET
),
data[9:],
)
elif 0x08 <= data[0] <= 0x2F:
value, remaining = data[0] - 8, data[1:]
add_to_object_list = False
Expand Down
9 changes: 5 additions & 4 deletions tests/support/test_opack.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ def test_pack_uuid():


def test_pack_absolute_time():
with pytest.raises(NotImplementedError):
pack(datetime.now())
assert pack(datetime.fromtimestamp(978307200.0)) == b"\x06" + (b"\x00" * 8)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same with the tests, I don't really like magic constants. I have used mach time in MRP IIRC, so you could add some kind of helper method to convert datetime objects mach time (and possibly pack) if you like.



def test_pack_small_integers():
Expand Down Expand Up @@ -223,8 +222,10 @@ def test_unpack_uuid():


def test_unpack_absolute_time():
# TODO: This is not implemented, it only parses the time stamp as an integer
assert unpack(b"\x06\x01\x00\x00\x00\x00\x00\x00\x00") == (1, b"")
assert unpack(b"\x06\x00\x00\x00\x00\x00\x00\x00\x00") == (
datetime.fromtimestamp(978307200.0),
b"",
)


def test_unpack_small_integers():
Expand Down