fix: reduce battery drain from polling in embedded mode#6
fix: reduce battery drain from polling in embedded mode#6KoheiYamashita merged 2 commits intomainfrom
Conversation
Stop health check polling after backend reaches RUNNING state, and replace cron service's 1-second ticker with timer-based scheduling that sleeps until the next job is due. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 組み込み環境、特にAndroidアプリのフォアグラウンドサービス内で動作するGoバックエンドにおいて、バッテリー消費の主要因となっていた2つのポーリング機構を最適化しました。これにより、CPUが不必要に起動し続けることを防ぎ、Dozeモードへの移行を妨げないことで、全体的なバッテリー寿命の改善に貢献します。 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
このプルリクエストは、ポーリングに起因するバッテリー消費を削減するための重要な修正です。Kotlin側のヘルスチェックの停止と、Go側のcronサービスをタイマーベースの実行に切り替える変更は、どちらも適切に実装されています。特に、GoのrunLoopにおけるtime.Timerとnilチャネルを組み合わせた省電力化の実装は、効率的でGoの言語特性をうまく活かしています。また、関連するテストケースが追加されている点も素晴らしいです。全体として、コードの品質は高く、問題は見つかりませんでした。
Add SIGTERM handling to Go gateway so proc.destroy() triggers graceful shutdown and immediate port release. Add Mutex to GatewayProcessManager to prevent concurrent start/stop races between watchdog and settings observer. Add port release polling after process termination. Catch InterruptedIOException in log forwarding to prevent crash on shutdown. Defer settingsStore.update() until after setup complete API returns to avoid killing the gateway mid-request. Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Merge duplicate parameter descriptions in buildParameters (#1) - Use map for DisabledActions lookup in isActionEnabled (#2) - Add URL scheme whitelist (http/https only) for open_url (#3) - Escape SQL LIKE wildcards in ContactsActionHandler (#4) - Fix JSON tag inconsistency: web_actions -> web (#5) - Add comprehensive tests for android tool (#6) Co-Authored-By: Claude Opus 4.6 <[email protected]>
* feat: extend android tool with category-based action system Split monolithic android tool into category-based modules (alarm, calendar, contacts, communication, media, navigation, device control, settings, web, clipboard) with per-category enable/disable config. Privacy-sensitive categories (contacts, communication) default to off. Closes #24 Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address PR review feedback - Merge duplicate parameter descriptions in buildParameters (#1) - Use map for DisabledActions lookup in isActionEnabled (#2) - Add URL scheme whitelist (http/https only) for open_url (#3) - Escape SQL LIKE wildcards in ContactsActionHandler (#4) - Fix JSON tag inconsistency: web_actions -> web (#5) - Add comprehensive tests for android tool (#6) Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: replace DisabledActions with struct-based per-action toggles Replace the DisabledActions string slice with typed action structs (AlarmActions, CalendarActions, etc.) grouped under AndroidActions. Each action is now a bool field with a label tag, so the schema system auto-generates individual toggles in the settings UI. - Add 10 category action structs + AndroidActions wrapper to config - Update migrateV2ToV3 to convert old DisabledActions to new struct - Add depth field to SchemaField for hierarchical UI rendering - Hide DisabledActions from UI via empty label tag - Update Android app to use depth-based indentation in config screen Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor: unify categories and actions into per-category structs Merge the separate Categories/Actions structure into unified category wrappers (e.g. AlarmCategory with Enabled + Actions). This makes each category toggle and its actions appear together in the settings UI with proper depth-based hierarchy. Android UI now disables child action toggles when the parent category is turned off, providing clear visual feedback. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: propagate section-level enabled toggle to all child fields When the top-level "Enabled" toggle (e.g. Android tool) is turned off, all child categories and actions are now greyed out in the settings UI. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: use depth instead of group to detect section-level toggle The section-level "Enabled" field (e.g. android.enabled) has a non-empty group ("Android"), so the previous group.isEmpty() check never matched. Use depth <= 1 to identify section toggles and depth > 1 for category toggles. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: add runtime permission requests before tool action execution Add permission checking/requesting before ActionHandler execution so users see permission dialogs instead of SecurityException errors. Runtime permissions (calendar, contacts, location) use PermissionRequester with suspendCancellableCoroutine to await the dialog result. Special permissions (DND, write settings) open the settings screen with an error message since they cannot be awaited programmatically. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: use ContentResolver.insert for create_event instead of Intent Replace the Intent-based calendar event creation (which opens the calendar app's add screen) with direct ContentResolver.insert() for consistency with update_event, delete_event, and add_reminder. Automatically selects the primary or first writable calendar as default. WRITE_CALENDAR permission is now required and requested via the permission dialog. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: add calendar picker for selecting target calendar account Allow users to choose which calendar to use for write actions (create_event, update_event, add_reminder) instead of always auto-detecting the primary calendar. On first use without a configured calendar, a picker dialog is shown and the selection is saved to config. The setting is also editable from the config UI. Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor: remove liveCfg from AndroidTool and redesign calendar picker with Compose Remove runtime config mutation (saveCalendarIdToConfig) from CalendarActionHandler — calendar_id is now a value copy from config, changed via settings screen and reflected on restart. Replace android.app.AlertDialog in CalendarPickerActivity with Compose AlertDialog using ClawDroidTheme colors. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: change calendar setting label from auto-detect to pick each time When calendar_id is empty, the picker launches every write action. The label should reflect this behavior instead of implying auto-detection. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: add input validation and race condition guards for security hardening - Add phone number and email validation (Go + Kotlin) to reject injection attempts - Validate intent extras to only allow primitive types (reject nested maps/arrays) - Return error for invalid settings sections instead of silent fallback - Add URL scheme validation (http/https only) in WebActionHandler - Fix BroadcastReceiver double-unregister race with AtomicBoolean guard - Add comprehensive tests for all new validations Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
📝 Description
組み込み版(embedded)で2つのポーリング機構がCPUを常時起こし続け、バッテリー消費の原因になっていた問題を修正。
GatewayProcessManager.pollHealth()がRUNNING状態遷移後も500ms間隔でHTTPポーリングを継続していたのを、RUNNING到達時点でコルーチンを終了するよう変更time.Timer+getNextWakeMS()で次のジョブの実行時刻まで正確にスリープする方式に変更。ジョブ0件時はCPU消費ゼロで完全スリープ🗣️ Type of Change
📚 Technical Context (Skip for Docs)
ヘルスチェック (Android/Kotlin)
pollHealth()でRUNNING遷移後にreturnを追加し、コルーチンを自然終了stopProcess()の既存キャンセル処理は完了済みJobに対してno-opなので安全Cronサービス (Go)
rescheduleCh chan struct{}(バッファ1) を追加し、ジョブの追加/削除/変更時にタイマーを即座に再計算nilチャネルのselectブロック特性を利用し、ジョブなし時は完全スリープAddJob,UpdateJob,RemoveJob,EnableJob,executeJobByIDの全mutationメソッドにnotifyReschedule()を追加🧪 Test Environment & Hardware
go test ./pkg/cron/ -v -count=1— 全29テスト PASS(既存27件 + 新規2件)☑️ Checklist
🤖 Generated with Claude Code