Virtual printer #1
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's Guide新增一个用于调试负载的 VirtualPrinter
|
| Change | Details | Files |
|---|---|---|
| 引入一个能够记录并打印接收到的负载用于调试的 VirtualPrinter 设备类。 |
|
unilabos/devices/virtual/virtual_printer.py |
在设备注册表中注册新的 virtual_printer 设备及其动作。 |
|
unilabos/registry/devices/virtual_device.yaml |
将各液体处理器设备中的 mix_times 配置和模式从单个整数修改为整型数组。 |
|
unilabos/registry/devices/liquid_handler.yamlunilabos/registry/devices/laiyu_liquid.yaml |
| 为测试添加占位/模拟 JSON 文件。 |
|
test/mock/laiyu.jsontest/mock/mock.json |
Tips and commands
与 Sourcery 交互
- 触发新的 Review： 在 Pull Request 中评论
@sourcery-ai review。 - 继续讨论： 直接回复 Sourcery 的 Review 评论。
- 从 Review 评论生成 GitHub issue： 在某条 Review 评论下回复，请 Sourcery 从该评论创建 issue。你也可以直接回复
@sourcery-ai issue，从该评论生成 issue。 - 生成 Pull Request 标题： 在 Pull Request 标题中任意位置写上
@sourcery-ai即可随时生成标题。你也可以在 PR 中评论@sourcery-ai title来（重新）生成标题。 - 生成 Pull Request 摘要： 在 Pull Request 正文任意位置写上
@sourcery-ai summary，即可在对应位置生成 PR 摘要。你也可以在 PR 中评论@sourcery-ai summary来（重新）生成摘要。 - 生成 Reviewer's Guide： 在 Pull Request 中评论
@sourcery-ai guide，即可随时（重新）生成 Reviewer's Guide。 - 一次性解决所有 Sourcery 评论： 在 Pull Request 中评论
@sourcery-ai resolve，将所有 Sourcery 评论标记为已解决。适用于你已经处理完所有评论但不想再看到它们的情况。 - 一次性关闭所有 Sourcery Reviews： 在 Pull Request 中评论
@sourcery-ai dismiss，以关闭所有现有的 Sourcery Review。若想重新开始新的 Review，别忘了再评论@sourcery-ai review触发新的 Review！
自定义你的体验
访问你的 dashboard 以：
- 启用或禁用诸如 Sourcery 自动生成的 Pull Request 摘要、Reviewer's Guide 等 Review 功能。
- 更改 Review 语言。
- 添加、删除或编辑自定义 Review 指南。
- 调整其他 Review 设置。
获取帮助
Original review guide in English
Reviewer's Guide
Adds a new virtual_printer device implementation and registry entry for debugging payloads, and adjusts several liquid handler schemas so mix_times is treated as an integer array instead of a scalar, also changing the virtual printer cleanup behavior to report false on completion.
Sequence diagram for VirtualPrinter print_message flow
sequenceDiagram
actor Client
participant VirtualPrinter
participant Logger
participant Console
Client->>VirtualPrinter: print_message(content, kwargs)
activate VirtualPrinter
VirtualPrinter->>VirtualPrinter: _record_and_print(action="print_message", content, kwargs)
activate VirtualPrinter
VirtualPrinter->>Console: print(prefix + " received:\n" + formatted_record)
VirtualPrinter->>Logger: info("Received: %s", record)
deactivate VirtualPrinter
VirtualPrinter-->>Client: { success: True, message: printed, return_info: printed }
deactivate VirtualPrinter
ER diagram for updated mix_times schema in liquid handlers
erDiagram
MIX_ACTION_SCHEMA {
float mix_liquid_height
int mix_rate
string mix_stage
int[] mix_times
int mix_vol
}
LIQUID_HANDLER {
string id
}
LIQUID_HANDLER_BIOMEK {
string id
}
LIQUID_HANDLER_PRCXI {
string id
}
LAIYU_LIQUID_HANDLER {
string id
}
LIQUID_HANDLER ||--o{ MIX_ACTION_SCHEMA : uses
LIQUID_HANDLER_BIOMEK ||--o{ MIX_ACTION_SCHEMA : uses
LIQUID_HANDLER_PRCXI ||--o{ MIX_ACTION_SCHEMA : uses
LAIYU_LIQUID_HANDLER ||--o{ MIX_ACTION_SCHEMA : uses
Class diagram for the new VirtualPrinter device
classDiagram
class VirtualPrinter {
- BaseROS2DeviceNode _ros_node
- str device_id
- dict config
- Logger logger
- dict data
- str port
- str prefix
- bool pretty
+ VirtualPrinter(device_id, config, kwargs)
+ post_init(ros_node)
+ initialize() bool
+ cleanup() bool
+ print_message(content, kwargs) dict
+ receive(args, kwargs) dict
- _record_and_print(action, content, kwargs) None
+ status() str
+ message() str
+ last_received() Any
+ received_count() int
}
class BaseROS2DeviceNode {
}
VirtualPrinter --> BaseROS2DeviceNode : uses
File-Level Changes
| Change | Details | Files |
|---|---|---|
| Introduce a VirtualPrinter device class that logs and prints received payloads for debugging. |
|
unilabos/devices/virtual/virtual_printer.py |
| Register the new virtual_printer device and its actions in the devices registry. |
|
unilabos/registry/devices/virtual_device.yaml |
| Change mix_times configuration and schema from a single integer to an array of integers across liquid handler devices. |
|
unilabos/registry/devices/liquid_handler.yamlunilabos/registry/devices/laiyu_liquid.yaml |
| Add placeholder/mock JSON files for tests. |
|
test/mock/laiyu.jsontest/mock/mock.json |
Tips and commands
Interacting with Sourcery
- Trigger a new review: Comment
@sourcery-ai reviewon the pull request. - Continue discussions: Reply directly to Sourcery's review comments.
- Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with@sourcery-ai issueto create an issue from it. - Generate a pull request title: Write
@sourcery-aianywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai titleon the pull request to (re-)generate the title at any time. - Generate a pull request summary: Write
@sourcery-ai summaryanywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment@sourcery-ai summaryon the pull request to
(re-)generate the summary at any time. - Generate reviewer's guide: Comment
@sourcery-ai guideon the pull
request to (re-)generate the reviewer's guide at any time. - Resolve all Sourcery comments: Comment
@sourcery-ai resolveon the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore. - Dismiss all Sourcery reviews: Comment
@sourcery-ai dismisson the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai reviewto trigger a new review!
Customizing Your Experience
Access your dashboard to:
- Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others. - Change the review language.
- Add, remove or edit custom review instructions.
- Adjust other review settings.
Getting Help
- Contact our support team for questions or feedback.
- Visit our documentation for detailed guides and information.
- Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - 我已经审阅了你的改动，这里是一些反馈：
- 在
VirtualPrinter.cleanup中，你总是返回False，而其他成功的操作（例如initialize和print_message）会用True表示成功，这会让整个 API 的语义不一致；建议在清理成功时返回True，并将False保留给失败的情况。 pretty标志是通过bool(self.config.get("pretty", True))推导出来的，这会把很多非布尔值（例如字符串 "false"）当作True；如果这个配置可能来自 JSON 或 YAML，建议将其规范化为严格的布尔值（显式检查True/False或常见的字符串等价形式）。- 这个类目前同时使用
print和 logger 输出；如果能统一改用 logger（可以为虚拟打印机配置专用的 handler/formatter），会让输出处理在不同环境下更一致、更容易控制。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- 在 `VirtualPrinter.cleanup` 中，你总是返回 `False`，而其他成功的操作（例如 `initialize` 和 `print_message`）会用 `True` 表示成功，这会让整个 API 的语义不一致；建议在清理成功时返回 `True`，并将 `False` 保留给失败的情况。
- `pretty` 标志是通过 `bool(self.config.get("pretty", True))` 推导出来的，这会把很多非布尔值（例如字符串 "false"）当作 `True`；如果这个配置可能来自 JSON 或 YAML，建议将其规范化为严格的布尔值（显式检查 `True`/`False` 或常见的字符串等价形式）。
- 这个类目前同时使用 `print` 和 logger 输出；如果能统一改用 logger（可以为虚拟打印机配置专用的 handler/formatter），会让输出处理在不同环境下更一致、更容易控制。
## Individual Comments
### Comment 1
<location> `unilabos/registry/devices/liquid_handler.yaml:4022-4026` </location>
<code_context>
mix_rate: 0
mix_stage: ''
- mix_times: 0
+ mix_times:
+ - 0
mix_vol: 0
</code_context>
<issue_to_address>
**issue (bug_risk):** 将 `mix_times` 从整数改为整型数组，可能会破坏那些期望标量值的现有使用方。
由于 schema/默认值现在变成了整型数组（`[0]`），请确认所有调用点、序列化逻辑以及任何外部集成（工作流、UI、设备驱动）都能正确处理数组形式以及任何遗留的标量值，或者为使用方文档化一个清晰的迁移路径。
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进之后的代码审查。
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- In
VirtualPrinter.cleanupyou always returnFalsewhile other successful operations (likeinitializeandprint_message) signal success withTrue, which makes the API semantics inconsistent; consider returningTrueon successful cleanup and reservingFalsefor failure cases. - The
prettyflag is derived viabool(self.config.get("pretty", True)), which will treat many non-boolean values (e.g. the string "false") asTrue; if this config can come from JSON or YAML, consider normalizing it to a strict boolean (checking explicitly forTrue/Falseor common string equivalents). - The class currently uses both
printand the logger for output; consolidating on the logger (optionally with a dedicated handler/formatter for the virtual printer) would make output handling more consistent and easier to control in different environments.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `VirtualPrinter.cleanup` you always return `False` while other successful operations (like `initialize` and `print_message`) signal success with `True`, which makes the API semantics inconsistent; consider returning `True` on successful cleanup and reserving `False` for failure cases.
- The `pretty` flag is derived via `bool(self.config.get("pretty", True))`, which will treat many non-boolean values (e.g. the string "false") as `True`; if this config can come from JSON or YAML, consider normalizing it to a strict boolean (checking explicitly for `True`/`False` or common string equivalents).
- The class currently uses both `print` and the logger for output; consolidating on the logger (optionally with a dedicated handler/formatter for the virtual printer) would make output handling more consistent and easier to control in different environments.
## Individual Comments
### Comment 1
<location> `unilabos/registry/devices/liquid_handler.yaml:4022-4026` </location>
<code_context>
mix_rate: 0
mix_stage: ''
- mix_times: 0
+ mix_times:
+ - 0
mix_vol: 0
</code_context>
<issue_to_address>
**issue (bug_risk):** Changing `mix_times` from an integer to an integer array may break existing consumers expecting a scalar.
Since the schema/default is now an integer array (`[0]`), please verify that all call sites, serializers, and any external integrations (workflows, UIs, device drivers) correctly handle the array form and any legacy scalar values, or document a clear migration path for consumers.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+4022
to
4026
| mix_times: | ||
| - 0 | ||
| mix_vol: 0 | ||
| none_keys: | ||
| - '' |
There was a problem hiding this comment.
issue (bug_risk): 将 mix_times 从整数改为整型数组，可能会破坏那些期望标量值的现有使用方。
由于 schema/默认值现在变成了整型数组（[0]），请确认所有调用点、序列化逻辑以及任何外部集成（工作流、UI、设备驱动）都能正确处理数组形式以及任何遗留的标量值，或者为使用方文档化一个清晰的迁移路径。
Original comment in English
issue (bug_risk): Changing mix_times from an integer to an integer array may break existing consumers expecting a scalar.
Since the schema/default is now an integer array ([0]), please verify that all call sites, serializers, and any external integrations (workflows, UIs, device drivers) correctly handle the array form and any legacy scalar values, or document a clear migration path for consumers.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
add virtual printer
cleanup function return false
Summary by Sourcery
添加一个虚拟打印机设备，并调整液体处理器混合作业的配置模式。
新功能：
VirtualPrinter设备，用于记录并打印接收到的调试消息，并添加相应的注册表定义和状态字段。缺陷修复：
mix_times从单个整数改为整数数组，以符合预期的模式用法。增强：
Original summary in English
Summary by Sourcery
Add a virtual printer device and adjust liquid handler mix configuration schemas.
New Features:
Bug Fixes:
Enhancements: