-
Notifications
You must be signed in to change notification settings - Fork 0
add custom command handler into command server #139
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
base: master
Are you sure you want to change the base?
Changes from all commits
48869f8
ec69a55
eec204d
150fd0f
b8eed9a
ca654b8
7c37161
828b1f9
ea5ed5d
1f077d6
4640a2d
2942320
936bc86
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,17 +1,24 @@ | ||||||
| import base64 | ||||||
| import copy | ||||||
| import io | ||||||
| import json | ||||||
| import os | ||||||
| import socket | ||||||
| from threading import Lock | ||||||
| from typing import Optional | ||||||
| import sys | ||||||
| from threading import Lock, Thread | ||||||
| from typing import Callable, Optional, TypedDict | ||||||
|
|
||||||
| from PIL.Image import Image as PIL_Image | ||||||
|
|
||||||
| from .schema.agent_app_protocol import CommandKind, CommandRequest, CommandResponse, Status | ||||||
| from .task import Isolated | ||||||
|
|
||||||
|
|
||||||
| class CustomCommandRequest(TypedDict): | ||||||
| id: str | ||||||
| payload: str | ||||||
|
|
||||||
|
|
||||||
| class CommandServer(Isolated): | ||||||
| sock_path: Optional[str] | ||||||
| img_lock: Lock | ||||||
|
|
@@ -26,7 +33,9 @@ class CommandServer(Isolated): | |||||
|
|
||||||
| """ | ||||||
|
|
||||||
| def __init__(self, sock_path: Optional[str] = None) -> None: | ||||||
| def __init__( | ||||||
| self, sock_path: Optional[str] = None, custom_command_handler: Optional[Callable[[CustomCommandRequest], str]] = None | ||||||
| ) -> None: | ||||||
| super().__init__() | ||||||
| self.sock_path = None | ||||||
| env = "ACTCAST_COMMAND_SOCK" | ||||||
|
|
@@ -37,6 +46,7 @@ def __init__(self, sock_path: Optional[str] = None) -> None: | |||||
| self.running = True | ||||||
| self.img_lock = Lock() | ||||||
| self.img = None | ||||||
| self.custom_command_handler = custom_command_handler | ||||||
|
|
||||||
| def run(self) -> None: | ||||||
| """Run and start the activity""" | ||||||
|
|
@@ -54,39 +64,40 @@ def run(self) -> None: | |||||
| while self.running: | ||||||
| try: | ||||||
| conn, _ = s.accept() | ||||||
| request, err = CommandRequest.parse(conn) | ||||||
|
|
||||||
| if request is None: | ||||||
| raise RuntimeError("couldn't parse a request from actcast agent: `CommandRequest.parse()` failed") | ||||||
|
|
||||||
| # FIXME: this error handling is unreachable because `CommandRequest.parse()` returns `None` if parsing fails | ||||||
| if err: | ||||||
| error_response = CommandResponse( | ||||||
| copy.copy(request.id_), | ||||||
| Status.GENERAL_ERROR, | ||||||
| b"", | ||||||
| ) | ||||||
| conn.sendall(error_response.to_bytes()) | ||||||
| conn.shutdown(socket.SHUT_RDWR) | ||||||
| conn.close() | ||||||
| continue | ||||||
| Thread(target=self._handle_request, args=(conn,), daemon=True).start() | ||||||
| except socket.timeout: | ||||||
| pass | ||||||
| except Exception as e: | ||||||
| print(f"Unexpected CommandServer error: {e}", file=sys.stderr, flush=True) | ||||||
| pass | ||||||
| os.remove(self.sock_path) | ||||||
|
|
||||||
| response = None | ||||||
| def _handle_request(self, conn: socket.socket) -> None: | ||||||
| try: | ||||||
| request = CommandRequest.parse(conn) | ||||||
| except Exception as e: | ||||||
| print(f"Failed to parse command request: {e}", file=sys.stderr, flush=True) | ||||||
| conn.shutdown(socket.SHUT_RDWR) | ||||||
| conn.close() | ||||||
| return | ||||||
|
|
||||||
| if request.kind == CommandKind.TAKE_PHOTO: | ||||||
| response = self._handle_take_photo(request) | ||||||
| response = None | ||||||
|
|
||||||
| if response is None: | ||||||
| conn.shutdown(socket.SHUT_RDWR) | ||||||
| conn.close() | ||||||
| continue | ||||||
| if request.kind == CommandKind.TAKE_PHOTO: | ||||||
| response = self._handle_take_photo(request) | ||||||
| elif request.kind == CommandKind.CHECK_CUSTOM_COMMAND_AVAILABILITY: | ||||||
| response = self._handle_custom_command_availability(request) | ||||||
| elif request.kind == CommandKind.CUSTOM_COMMAND: | ||||||
| response = self._handle_custom_command(request) | ||||||
|
|
||||||
| conn.sendall(response.to_bytes()) | ||||||
| conn.shutdown(socket.SHUT_RDWR) | ||||||
| conn.close() | ||||||
| except socket.timeout: | ||||||
| pass | ||||||
| os.remove(self.sock_path) | ||||||
| if response is None: | ||||||
| conn.shutdown(socket.SHUT_RDWR) | ||||||
| conn.close() | ||||||
| return | ||||||
|
|
||||||
| conn.sendall(response.to_bytes()) | ||||||
| conn.shutdown(socket.SHUT_RDWR) | ||||||
| conn.close() | ||||||
|
|
||||||
| def _handle_take_photo(self, request: CommandRequest) -> Optional[CommandResponse]: | ||||||
| # Wait photo | ||||||
|
|
@@ -104,6 +115,32 @@ def _handle_take_photo(self, request: CommandRequest) -> Optional[CommandRespons | |||||
|
|
||||||
| return None | ||||||
|
|
||||||
| def _handle_custom_command_availability(self, request: CommandRequest) -> CommandResponse: | ||||||
| if self.custom_command_handler is None: | ||||||
| return CommandResponse(copy.copy(request.id_), Status.UNIMPLEMENTED, b"Custom command handler is not set") | ||||||
| else: | ||||||
| return CommandResponse(copy.copy(request.id_), Status.OK, b"") | ||||||
|
|
||||||
| def _handle_custom_command(self, request: CommandRequest) -> CommandResponse: | ||||||
| if self.custom_command_handler is None: | ||||||
| return CommandResponse( | ||||||
| copy.copy(request.id_), Status.GENERAL_ERROR, b"Unreachable Error: custom command handler is not set" | ||||||
|
Member
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.
Contributor
Author
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. 基本的にはここには入ってきてはいけないという意味で このコメントとしてはProtocol Error に変更してエラーメッセージを変えるべき的なことですかね👀 |
||||||
| ) | ||||||
|
|
||||||
| try: | ||||||
| command_server_request: CustomCommandRequest = json.loads(request.data.decode()) | ||||||
|
Member
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. json.loads は Python の型をランタイムで確認しないはずなので、ここで保証されるのは json として valid なことまでな気がします。 (あと、ここで落ちる場合は agent がおかしい場合だと思うため、エラーメッセージではユーザーにとっては対処のしようがないということは明示しておきたい気がします)
Contributor
Author
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.
はい、これは基本的に payload が JSON であるということのみを確認していて, ここで落ちるということは agent の request の形式がおかしいので GENERAL_ERROR として返してます!
基本的には我々がバグを発見するためのもので, ユーザーに何かを求める用途でないのはそうです.
Contributor
Author
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. と思ったけど、エラーメッセージなんてあっても別に困らないし書いてもいい気がしてきた なんなら agent が JSON を渡すことを前提に try-catch しないでもありかもしれない. |
||||||
| except Exception as e: | ||||||
| return CommandResponse( | ||||||
| copy.copy(request.id_), Status.GENERAL_ERROR, f"Failed to parse custom command payload: {e}".encode() | ||||||
|
Member
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. Error の種類が分かるようにしたいです。
Suggested change
|
||||||
| ) | ||||||
|
|
||||||
| try: | ||||||
| payload = self.custom_command_handler(command_server_request) | ||||||
| return CommandResponse(copy.copy(request.id_), Status.OK, payload.encode()) | ||||||
| except Exception as e: | ||||||
| error_message = str(e) | ||||||
|
Member
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. Error の種類が分かるようにしたいです。
Suggested change
|
||||||
| return CommandResponse(copy.copy(request.id_), Status.APP_ERROR, error_message.encode()) | ||||||
|
|
||||||
| def update_image(self, image: PIL_Image) -> None: | ||||||
| """ | ||||||
|
|
||||||
|
|
||||||
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.
typing_extensionsが pyproject.toml に入っていなそうなため、直接の依存として入れた方が壊れにくくなるかもしれません