feat(reminder): add interval time, expo-audio and vibration adapters - #215
feat(reminder): add interval time, expo-audio and vibration adapters#215gac0812 wants to merge 17 commits into
Conversation
Introduce TimeflowAlarmBridge and NativeAlarmScheduler, and wire the composition root alarms port to the native Android scheduler.
Introduce NativeDeviceCapability over TimeflowAlarmBridge and a launch permission prompt hook, and wire the composition root device port.
Wire IntervalTimeListener, ExpoAudioPlayback, and ReactNativeVibration into the composition root, and add the expo-audio dependency.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Return AlarmScheduleReceipt.scheduled, map native schedule rejections to unscheduled, and propagate the native cancel boolean instead of always reporting success.
…e-alarm Keep NativeAlarmScheduler wiring while adopting the auth runtime composition from 1024XEngineer#218.
…evice-permissions Keep NativeDeviceCapability wiring while adopting the auth runtime from 1024XEngineer#218.
…nder-playback-adapters Keep playback adapters while adopting the auth runtime from 1024XEngineer#218.
… feat/reminder-native-device-permissions
…issions' into feat/reminder-playback-adapters
…ack-adapters Keep IntervalTimeListener, ExpoAudioPlayback, and ReactNativeVibration wiring while adopting 1024XEngineer#208 device-capability fixes and expo-location from main.
There was a problem hiding this comment.
Found one integration issue in the current diff: the new production adapters are injected, but the composition root still constructs the mock reminder application, so the adapters are not reached by the running reminder flow. The review comment identifies the concrete impact and the required integration point.
Verification: repository was locked to the requested base/head SHAs. Local validation could not run because the mounted frontend/node_modules is incomplete (tsc is missing/permission-denied).
… switch MockReminderApplication never invokes time/audio/vibration, so wiring production adapters there had no runtime effect.
|
@fennoai /review |
Review Complete
No noteworthy issues found. The non-blocking PR review was submitted with the same conclusion. Verification: |
|
【Issue 验收不一致 / 阻塞合并】PR 使用 |
|
【PR 描述需同步最终实现】Summary 仍写“新增适配器”,但早期描述/合并顺序容易让 reviewer 误以为它们已接入运行时;最新代码实际上只新增未接线的适配器与单测。 |
Wintercom
left a comment
There was a problem hiding this comment.
按照 git_rules.txt 的合并标准,本轮需要先处理以下阻塞项:
- PR 使用
Closes #214,但最新实现明确未把time/audio/vibration接入组合根,与 Issue #214 的核心验收项不一致;请改为阶段性交付并保持 Issue 开放,或补齐真实接线与集成测试。 ExpoAudioPlayback会吞掉 audio mode 设置失败,并在没有确认实际播放状态时返回played: true;player 创建/replace/play 失败也无法稳定触发本地 fallback。- PR 描述需要明确当前适配器不可从生产提醒流程到达,并指出后续接线 Issue/PR 与本次验证边界。
具体内容已使用 gh 逐条评论。此前 reviewer 关于 mock 组合根导致生产适配器不可达的问题虽已通过撤销接线规避,但由此产生的 Issue 验收不一致仍需处理。
Wait for expo-audio playing status (or a timeout/error) before returning played: true, and treat mode/replace/play exceptions as unplayed.
|
@fennoai /review |
Review Complete
Found one P2 cleanup issue: when native Verification: focused playback suite passed (10 tests); full |
|
已处理 Wintercom 本轮阻塞项:
|
The confirmation timer and status listener must be torn down on the play() catch path so failed playback does not leak a 2s handle.
Summary
IntervalTimeListener、ExpoAudioPlayback(含audioDataUri)、ReactNativeVibration,并引入expo-audio依赖与类型声明ExpoAudioPlayback仅在playbackStatusUpdate确认playing后返回played: true;setAudioModeAsync/replace/play失败或超时则为false,避免假成功挡住本地 fallbackcreateAppServices仍使用MockTimeListener/MockAudioPlayback/MockVibration:当前组合根是MockReminderApplication,不会调用这些端口,生产提醒流程到不了本 PR 的适配器time/audio/vibration接线仍留在 feat(reminder): 接入时间/音频/震动原生播放适配器 #214Merge Order
Scope
frontend/src/shared/time/IntervalTimeListener.tsfrontend/src/shared/time/index.tsfrontend/src/infrastructure/audio/ExpoAudioPlayback.tsfrontend/src/infrastructure/audio/audioDataUri.tsfrontend/src/infrastructure/audio/index.tsfrontend/src/infrastructure/notifications/ReactNativeVibration.tsfrontend/src/infrastructure/notifications/index.tsfrontend/package.json/frontend/package-lock.jsonfrontend/src/types/expo-audio.d.tsfrontend/tsconfig.jsonfrontend/tests/unit/shared/time/intervalTimeListener.test.tsfrontend/tests/unit/infrastructure/audio/audioDataUri.test.tsfrontend/tests/unit/infrastructure/audio/expoAudioPlayback.test.tsfrontend/tests/unit/infrastructure/notifications/reactNativeVibration.test.tsOut of scope
time/audio/vibration切到本 PR 适配器(需先有LocalReminderApplication,见 feat(reminder): wire LocalReminderApplication with in-memory adapters #238;收尾仍属 feat(reminder): 接入时间/音频/震动原生播放适配器 #214)timeflow-alarm模块本体Verification
npm run checknpm ciRelated to #214