Skip to content

Add file_client and file_server examples#391

Open
maksimdrachov wants to merge 1 commit into
masterfrom
add-file-example
Open

Add file_client and file_server examples#391
maksimdrachov wants to merge 1 commit into
masterfrom
add-file-example

Conversation

@maksimdrachov

Copy link
Copy Markdown
Member

No description provided.

@pavel-kirienko pavel-kirienko left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please bump the dev version number so that it's published

Comment thread examples/file_client.py
Comment on lines +27 to +30
REQUEST_DELIVERY_TIMEOUT = RESPONSE_TIMEOUT / 2.0
REQUEST_HEADER_FORMAT = "<QH"
RESPONSE_HEADER_FORMAT = "<IH"
RESPONSE_HEADER_SIZE = struct.calcsize(RESPONSE_HEADER_FORMAT)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These belong in the dataclasses and they should be private with exposed serialize/deserialize methods. Maybe we should even move them into a shared module to reduce duplication between the examples.

Comment thread examples/file_client.py
data: bytes


def _encode_request(file_path: str, read_offset: int) -> bytes:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same, broken encapsulation

Comment thread examples/file_server.py
if request is None:
_logger.debug("dropping malformed request of size %d", len(arrival.message))
continue
task = asyncio.create_task(_serve_request(arrival, request), name=f"file:{arrival.breadcrumb.tag}")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this example would benefit from doing it sequentially to focus more on Cyphal rather than asyncio.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants