-
Notifications
You must be signed in to change notification settings - Fork 9
Develop #123
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?
Develop #123
Changes from all commits
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,5 +1,19 @@ | ||
| # Audio Stream Store Refactoring | ||
|
|
||
| ## Expo SDK 56 migration requirement | ||
|
|
||
| Before upgrading Dispatch to Expo SDK 56, upgrade `expo-audio` to the SDK 56-compatible version and replace all remaining `expo-av` audio usage with `expo-audio`. SDK 56 no longer provides the legacy Expo Modules Core header required by `expo-av` 16, so leaving `expo-av` installed can break the iOS archive build. | ||
|
|
||
| Migration checklist: | ||
|
|
||
| - Migrate `src/hooks/use-ptt.ts`, `src/components/calls/call-audio-modal.tsx`, `src/stores/app/audio-stream-store.ts`, and `src/services/audio.service.ts` to `createAudioPlayer`, `setAudioModeAsync`, `AudioPlayer`, and `playbackStatusUpdate`. | ||
| - Remove `expo-av` from `package.json`, the lockfile, tests/mocks, and the Expo Doctor exclusion after no imports remain. | ||
|
Comment on lines
+9
to
+10
Contributor
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. 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== file excerpt =="
if [ -f docs/audio-stream-refactoring.md ]; then
wc -l docs/audio-stream-refactoring.md
sed -n '1,220p' docs/audio-stream-refactoring.md
else
echo "docs/audio-stream-refactoring.md not found"
fi
echo
echo "== expo-av mentions in docs/audio-stream-refactoring.md =="
rg -n "expo-av|expo-audio|audio-av|createAudioPlayer|setAudioModeAsync|AudioPlayer|playbackStatusUpdate|Migrate|Remove" docs/audio-stream-refactoring.md || trueRepository: Resgrid/Dispatch Length of output: 7247 🌐 Web query:
💡 Result: In Expo SDK 56, the expo-audio library is the standard tool for audio playback and recording, having replaced the deprecated expo-av package [1][2]. To work with audio, you primarily use the useAudioPlayer hook or, in advanced scenarios where the player must persist beyond a component's lifecycle, the createAudioPlayer function [3][4]. 1. useAudioPlayer(source, options): This is the recommended approach for most use cases [3]. It is a React hook that automatically manages the lifecycle of the AudioPlayer instance, ensuring that resources are properly disposed of when the component unmounts [5][6]. 2. createAudioPlayer(source, options): This function allows you to create an AudioPlayer instance manually [3][5]. Because this instance is not automatically managed by a component's lifecycle, it is your responsibility to call the release method on the player when it is no longer needed to prevent memory leaks [3][4]. Key differences when migrating from expo-av include: - Time is measured in seconds rather than milliseconds [7]. - Audio loads immediately upon creation [7]. - There is no automatic reset to the beginning when playback finishes; if you wish to replay, you must manually call player.seekTo(0) before calling play [7][8]. For additional functionality, expo-audio also provides the useAudioRecorder hook for recording [7][3]. Citations:
Align Lines 9-10 remove 🤖 Prompt for AI AgentsSource: MCP tools |
||
| - Keep this as an audio-only migration. Dispatch does not currently use the `expo-av` video component, so `expo-video` is not required for this change. | ||
| - Re-test remote MP3 streams on physical iOS and Android devices. This store originally moved to `expo-av` because remote streams had problems with the earlier `expo-audio` implementation. | ||
| - Also test PTT, call audio, background playback, interruptions, and Bluetooth/headset routing before release. | ||
|
|
||
| Do not copy an SDK 56 implementation back into the current SDK 54 app unchanged. Dispatch's current `expo-audio` 1.1 API does not expose SDK 56 options such as `preferredForwardBufferDuration` or playback `status.error`. | ||
|
|
||
| ## Overview | ||
|
|
||
| The audio stream store has been refactored to use `expo-av` instead of `expo-audio` to resolve issues with playing remote MP3 streams over the internet in the new Expo architecture. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| import { api } from '../common/client'; | ||
|
|
||
| const FEATURE_TOGGLES = '/FeatureToggles'; | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // Feature toggle evaluation (department-scoped, any authenticated user). | ||
| // Backed by the v4 FeatureToggles API; keys live in Resgrid.Model.FeatureFlagKeys. | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| export interface FeatureToggleData { | ||
| Key: string; | ||
| Enabled: boolean; | ||
| Value?: string | null; | ||
| ValueType?: string | null; | ||
| Source?: string | null; | ||
| } | ||
|
|
||
| export interface FeatureTogglesResult { | ||
| Data?: FeatureToggleData[]; | ||
| StateHash?: string; | ||
| } | ||
|
|
||
| export interface FeatureToggleResult { | ||
| Data?: FeatureToggleData; | ||
| } | ||
|
|
||
| /** Evaluates every active flag for the caller's department. */ | ||
| export const getAllFeatureFlags = async (signal?: AbortSignal) => { | ||
| const response = await api.get<FeatureTogglesResult>(`${FEATURE_TOGGLES}/GetAll`, { signal }); | ||
|
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. Unguarded external HTTP call via Kody rule violation: Add try-catch blocks for external calls Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| return response.data; | ||
| }; | ||
|
|
||
| /** Lightweight enabled-only check for a single flag. */ | ||
| export const getFeatureFlagState = async (key: string, signal?: AbortSignal) => { | ||
| const response = await api.get<FeatureToggleResult>(`${FEATURE_TOGGLES}/GetState`, { params: { key }, signal }); | ||
| return response.data; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,7 @@ import { type GetConfigResultData } from '@/models/v4/configs/getConfigResultDat | |
| import { usePushNotifications } from '@/services/push-notification'; | ||
| import { useCoreStore } from '@/stores/app/core-store'; | ||
| import { useCallsStore } from '@/stores/calls/store'; | ||
| import { FeatureFlagKeys, featureFlagsStore } from '@/stores/feature-flags/store'; | ||
| import useLockscreenStore from '@/stores/lockscreen/store'; | ||
| import { useRolesStore } from '@/stores/roles/store'; | ||
| import { securityStore } from '@/stores/security/store'; | ||
|
|
@@ -150,7 +151,14 @@ export default function TabLayout() { | |
| await securityStore.getState().getRights(); | ||
|
|
||
| logger.info({ | ||
| message: 'Security rights retrieved, connecting SignalR', | ||
| message: 'Security rights retrieved, fetching feature flags', | ||
| context: { platform: Platform.OS }, | ||
| }); | ||
|
|
||
| await featureFlagsStore.getState().fetchFlags(); | ||
|
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. Unguarded awaited Kody rule violation: Handle async operations with proper error handling Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
|
|
||
| logger.info({ | ||
| message: 'Feature flags fetched, connecting SignalR', | ||
| context: { platform: Platform.OS }, | ||
| }); | ||
|
|
||
|
|
@@ -169,18 +177,26 @@ export default function TabLayout() { | |
| // Don't fail initialization if SignalR connection fails | ||
| } | ||
|
|
||
| // Connect the realtime chat hub (best-effort; chat may be disabled per department) | ||
| try { | ||
| await useSignalRStore.getState().connectChatHub(); | ||
| // Connect the realtime chat hub only when the Chat.System feature flag is on for | ||
|
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. Feature flag bypass occurs in // Gate must also be enforced in use-signalr-lifecycle.ts handleAppResume:
// const hubs = [signalRStore.connectUpdateHub(), signalRStore.connectGeolocationHub()];
// if (featureFlagsStore.getState().isEnabled(FeatureFlagKeys.ChatSystem)) {
// hubs.push(signalRStore.connectChatHub());
// }
// const results = await Promise.allSettled(hubs);Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| // this department; when it is off every chat surface stays hidden. | ||
| if (featureFlagsStore.getState().isEnabled(FeatureFlagKeys.ChatSystem)) { | ||
| try { | ||
| await useSignalRStore.getState().connectChatHub(); | ||
| logger.info({ | ||
| message: 'SignalR chat hub connected successfully', | ||
| context: { platform: Platform.OS }, | ||
| }); | ||
| } catch (error) { | ||
| logger.error({ | ||
| message: 'Failed to connect SignalR chat hub during initialization', | ||
| context: { error, platform: Platform.OS }, | ||
| }); | ||
| } | ||
| } else { | ||
| logger.info({ | ||
| message: 'SignalR chat hub connected successfully', | ||
| message: 'Chat disabled by feature flag; skipping chat hub connection', | ||
| context: { platform: Platform.OS }, | ||
| }); | ||
| } catch (error) { | ||
| logger.error({ | ||
| message: 'Failed to connect SignalR chat hub during initialization', | ||
| context: { error, platform: Platform.OS }, | ||
| }); | ||
| } | ||
|
|
||
| // Initialize weather alerts | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import { type Href, Stack, useFocusEffect, useRouter } from 'expo-router'; | ||
| import { type Href, Redirect, Stack, useFocusEffect, useRouter } from 'expo-router'; | ||
| import { Bot, MessageCircle, MessagesSquare, Network, Plus, Sparkles, Users } from 'lucide-react-native'; | ||
| import React, { useCallback, useState } from 'react'; | ||
| import { useTranslation } from 'react-i18next'; | ||
|
|
@@ -15,17 +15,19 @@ | |
| import { FocusAwareStatusBar } from '@/components/ui/focus-aware-status-bar'; | ||
| import { HStack } from '@/components/ui/hstack'; | ||
| import { Pressable } from '@/components/ui/pressable'; | ||
| import { Spinner } from '@/components/ui/spinner'; | ||
| import { Text } from '@/components/ui/text'; | ||
| import { VStack } from '@/components/ui/vstack'; | ||
| import { type ChatChannelResultData, ChatChannelType } from '@/models/v4/chat'; | ||
| import { useChatStore } from '@/stores/chat/store'; | ||
| import { useChatSystemStatus } from '@/stores/feature-flags/store'; | ||
|
|
||
| function ChannelRow({ channel, onPress }: { channel: ChatChannelResultData; onPress: () => void }) { | ||
| const { t } = useTranslation(); | ||
| const unread = channel.UnreadCount > 0; | ||
| const isDm = channel.ChannelType === ChatChannelType.DirectMessage; | ||
|
|
||
| const Leading = () => { | ||
|
Check warning on line 30 in src/app/(app)/chat.tsx
|
||
| if (isDm) { | ||
| return ( | ||
| <Avatar size="md"> | ||
|
|
@@ -81,6 +83,8 @@ | |
| export default function ChatScreen() { | ||
| const { t } = useTranslation(); | ||
| const router = useRouter(); | ||
| const chatStatus = useChatSystemStatus(); | ||
| const isChatEnabled = chatStatus === 'enabled'; | ||
| const channels = useChatStore((s) => s.channels); | ||
| const isLoading = useChatStore((s) => s.isLoadingChannels); | ||
| const pendingAcks = useChatStore((s) => s.pendingAcks); | ||
|
|
@@ -89,9 +93,10 @@ | |
|
|
||
| useFocusEffect( | ||
| useCallback(() => { | ||
| if (!isChatEnabled) return; | ||
| useChatStore.getState().fetchChannels(); | ||
| useChatStore.getState().fetchPendingAcks(); | ||
| }, []) | ||
| }, [isChatEnabled]) | ||
| ); | ||
|
|
||
| const grouped = groupChannels(channels); | ||
|
|
@@ -103,6 +108,22 @@ | |
| [router] | ||
| ); | ||
|
|
||
| // Chat.System flag not yet resolved: wait instead of redirecting away from a valid route. | ||
| if (chatStatus === 'unknown') { | ||
| return ( | ||
| <Box className="size-full flex-1 items-center justify-center bg-background-0"> | ||
| <Stack.Screen options={{ headerShown: false }} /> | ||
| <FocusAwareStatusBar /> | ||
| <Spinner /> | ||
| </Box> | ||
| ); | ||
| } | ||
|
|
||
| // Chat.System feature flag off: no chat for this department. | ||
| if (chatStatus === 'disabled') { | ||
| return <Redirect href={'/home' as Href} />; | ||
|
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. Magic string '/home' for a finite application route reduces maintainability and risks typos. Define route names as a Route enum or const tuple instead of using raw strings. Kody rule violation: Use enums instead of magic strings Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| } | ||
|
|
||
| return ( | ||
| <Box className="size-full flex-1 bg-background-0"> | ||
| <Stack.Screen options={{ headerShown: false }} /> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| import { Image } from 'expo-image'; | ||
| import { type Href, Stack, useFocusEffect, useLocalSearchParams, useRouter } from 'expo-router'; | ||
| import { type Href, Redirect, Stack, useFocusEffect, useLocalSearchParams, useRouter } from 'expo-router'; | ||
| import { Circle } from 'lucide-react-native'; | ||
| import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; | ||
| import { useTranslation } from 'react-i18next'; | ||
|
|
@@ -27,6 +27,7 @@ import { VStack } from '@/components/ui/vstack'; | |
| import { ChatChannelType, ChatMessagePriority, type ChatMessageResultData, ChatMessageType, type GifResultData } from '@/models/v4/chat'; | ||
| import useAuthStore from '@/stores/auth/store'; | ||
| import { useChatStore } from '@/stores/chat/store'; | ||
| import { useChatSystemStatus } from '@/stores/feature-flags/store'; | ||
| import { securityStore } from '@/stores/security/store'; | ||
| import { useToastStore } from '@/stores/toast/store'; | ||
|
|
||
|
|
@@ -38,6 +39,8 @@ export default function ChannelConversationScreen() { | |
|
|
||
| const currentUserId = useAuthStore((s) => s.userId); | ||
| const isModerator = !!securityStore((s) => s.rights)?.IsAdmin; | ||
| const chatStatus = useChatSystemStatus(); | ||
| const isChatEnabled = chatStatus === 'enabled'; | ||
|
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. Magic string comparison introduces silent failures if the store value changes or a typo occurs. Define a const tuple like Kody rule violation: Use enums instead of magic strings Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
|
|
||
| const channel = useChatStore((s) => s.channels.find((c) => c.ChatChannelId === channelId)); | ||
| const messages = useChatStore((s) => (channelId ? s.messagesByChannel[channelId] : undefined)); | ||
|
|
@@ -64,7 +67,7 @@ export default function ChannelConversationScreen() { | |
| // Mount: activate channel, join hub, load history and members. | ||
| useFocusEffect( | ||
| useCallback(() => { | ||
| if (!channelId) return; | ||
| if (!channelId || !isChatEnabled) return; | ||
| const store = useChatStore.getState(); | ||
| store.setActiveChannel(channelId); | ||
| void store.joinChannel(channelId); | ||
|
|
@@ -73,7 +76,7 @@ export default function ChannelConversationScreen() { | |
| return () => { | ||
| useChatStore.getState().setActiveChannel(null); | ||
| }; | ||
| }, [channelId]) | ||
| }, [channelId, isChatEnabled]) | ||
| ); | ||
|
|
||
| // Fetch presence for the channel members (for the header online dot). | ||
|
|
@@ -236,6 +239,21 @@ export default function ChannelConversationScreen() { | |
|
|
||
| const title = channel ? getChannelDisplayName(channel, t) : t('chat.title'); | ||
|
|
||
| // Chat.System flag not yet resolved: wait instead of redirecting away from a valid deep link. | ||
| if (chatStatus === 'unknown') { | ||
| return ( | ||
| <Box className="size-full flex-1 items-center justify-center bg-background-0"> | ||
| <Stack.Screen options={{ title, headerShown: true, headerBackTitle: '' }} /> | ||
| <Spinner /> | ||
| </Box> | ||
| ); | ||
| } | ||
|
|
||
| // Chat.System feature flag off: block deep links (push notifications, stale routes). | ||
| if (chatStatus === 'disabled') { | ||
| return <Redirect href={'/home' as Href} />; | ||
|
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. Hardcoded route path '/home' scatters definitions, causing renames or typos to evade compile-time checks. Reference a centralized routes constant, such as Kody rule violation: Centralize string constants Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| } | ||
|
|
||
| return ( | ||
| <Box className="size-full flex-1 bg-background-0"> | ||
| <Stack.Screen | ||
|
|
||
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.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: Resgrid/Dispatch
Length of output: 433
🏁 Script executed:
Repository: Resgrid/Dispatch
Length of output: 27844
🌐 Web query:
Expo SDK 54 expo-audio 1.1 installable expo-audio 16 expo-audio SDK 56 compatibility expo doc💡 Result:
The expo-audio library is fully compatible with and designed for modern Expo SDKs, including SDK 54 and SDK 56. 1. Expo SDK 54: expo-audio 1.1 is compatible and was the standard for audio in SDK 54 [1][2]. During the SDK 54 lifecycle, users were encouraged to migrate to expo-audio from the deprecated expo-av [3][4]. You can install the version appropriate for your SDK using the command: npx expo install expo-audio [5]. 2. Expo SDK 56: expo-audio version 56.x is the version associated with SDK 56 [6][7]. The library received significant updates in SDK 56, including a new useAudioStream hook and various live-streaming improvements [8][9]. Important Note on Versions: In the Expo ecosystem, you should not manually choose a version number like 16 for expo-audio unless it specifically aligns with your SDK. Instead, always use the command npx expo install expo-audio [5]. This command ensures that the Expo CLI automatically installs the specific version of the package that is tested and compatible with your current Expo SDK version [5]. Using version 16 with SDK 56 is likely incorrect, as the package versions for expo-audio follow the SDK release numbering (e.g., version 56.x for SDK 56) [6][7]. Always verify your dependencies using npx expo-doctor after installation [5].
Citations:
🌐 Web query:
docs.expo.dev "expo-audio" "Expo SDK 56"💡 Result:
In Expo SDK 56, the expo-audio library is officially supported and includes several key updates [1][2]. Key features and improvements for expo-audio in SDK 56 include: - New useAudioStream Hook: This hook provides real-time access to microphone buffer data, enabling new possibilities for audio processing [1][3]. - Live-Stream Enhancements: - An isLiveStream option is now available for the lock-screen on iOS [1][2]. - A playsInSilentMode option has been added for Android [1][3]. - The AudioStatus object has been updated to include new fields: isLive, currentOffsetFromLive, and error [1][2]. You can access the official documentation for this version at https://docs.expo.dev/versions/v56.0.0/sdk/audio/ [1][2].
Citations:
🌐 Web query:
Expo docs "Before upgrading" "During the upgrade" package compatibility SDK 56 expo install💡 Result:
To upgrade your project to Expo SDK 56, follow these recommended practices regarding preparation, package compatibility, and the installation process [1][2][3]. Preparation and Best Practices Before starting the upgrade, it is strongly recommended that you read the official SDK 56 changelog and breaking changes documentation [1][4]. This allows you to identify necessary configuration adjustments specific to your application before beginning the actual upgrade process [1]. Additionally, use a dedicated git branch for the upgrade to isolate changes [2][3]. If you are still using Expo Go for development, it is recommended to migrate to development builds first, as this provides a more stable baseline for testing and debugging during and after the upgrade [5]. It is also advised to upgrade one major change at a time (e.g., upgrading the SDK version first, then enabling the New Architecture separately) to make it easier to isolate and troubleshoot any issues [5][4][6]. Package Compatibility and Installation During the upgrade, you should prioritize ensuring that your dependencies are compatible with the new SDK version [2][3]. The primary command for updating your Expo-related packages is [2][3]: npx expo install expo@^56.0.0 After updating the core expo package, run the package-aware installer to automatically align your remaining dependencies with the requirements of SDK 56 [2][3]: npx expo install --fix Post-Upgrade Verification Once the installation is complete, it is crucial to verify your project's health. The recommended next step is to run Expo Doctor, which is designed to detect mismatched versions, incompatible dependencies, and configuration issues [2][3]: npx expo-doctor If your project utilizes Expo Router, be aware that SDK 56 includes internal migration work; if you encounter issues, you may need to use the provided codemod to handle transitions between file-based routing and React Navigation imports [2][3]: npx expo-codemod sdk-56-expo-router-react-navigation-replace src Finally, rebuild your development builds and test thoroughly before deploying to production [2][3].
Citations:
Sequence the SDK 56 package update with the SDK 56 upgrade.
Expo’s SDK upgrade process updates Expo packages with the SDK and uses
npx expo install --fixto align dependencies. Keep this guidance focused on the SDK upgrade, or change “Before upgrading” to “During the SDK 56 upgrade” and add the testedexpo-audiopackage version.🤖 Prompt for AI Agents
Source: MCP tools