Skip to content

Conversation

JakubAndrysek
Copy link
Contributor

This pull request adds a new set of framebuffer utilities to the client, making it easier to interact with devices that expose a framebuffer (such as LCD modules). The main changes include introducing a new framebuffer.py module with helper functions for reading, saving, and comparing framebuffer images, and exposing these helpers through the main client interface.

Copy link
Contributor

@urish urish left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I've left some comments, please review

@@ -233,3 +240,23 @@ async def set_control(
value: Control value to set (float).
"""
return await set_control(self._transport, part=part, control=control, value=value)

async def framebuffer_read(self, id: str) -> ResponseMessage:
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the use case for this API?

IMHO if there's no good use case, let's not expose it, since it is can be confusing when you have both framebuffer_read and framebuffer_png_bytes.


async def compare_framebuffer_png(
self, id: str, reference: Path, save_mismatch: Optional[Path] = None
) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO that doesn't belong to the API - it's better if users implement their own comparison logic as they want (and byte-by-byte comparison may break when we upgrade our PNG library, for example).

"""Read the current framebuffer for the given device id."""
return await framebuffer_read(self._transport, id=id)

async def framebuffer_png_bytes(self, id: str) -> bytes:
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's keep the naming convention consistent. If other methods are called "save_framebuffer_...", then this one should be "get_framebuffer_png_bytes" (or read_...)

@JakubAndrysek JakubAndrysek requested a review from urish August 20, 2025 11:34
@urish urish merged commit 111d347 into wokwi:main Aug 20, 2025
0 of 5 checks passed
@JakubAndrysek JakubAndrysek deleted the add-framebuffer branch August 20, 2025 13:45
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