Address code review

config.js

@@ -73,6 +73,11 @@ var config = {
     // Disable measuring of audio levels.
     // disableAudioLevels: false,
 
+    // Enabling this will run the lib-jitsi-meet no audio detection module which
+    // will notify the user if the current selected microphone has no audio
+    // input and will suggest another valid device if one is present.
+    // enableNoAudioDetection: false
+
     // Start the conference in audio only mode (no video is being received nor
     // sent).
     // startAudioOnly: false,
@@ -470,7 +475,6 @@ var config = {
      disableNS
      enableLipSync
      enableTalkWhileMuted
-     enableNoAudioDetection
      forceJVB121Ratio
      hiddenDomain
      ignoreStartMuted

react/features/no-audio-signal/middleware.js

@@ -1,7 +1,5 @@
 // @flow
-import { setNoAudioSignalNotificationUid } from './actions';
+
-import { NO_AUDIO_SIGNAL_SOUND_ID } from './constants';
-import { NO_AUDIO_SIGNAL_SOUND_FILE } from './sounds';
 import { APP_WILL_MOUNT, APP_WILL_UNMOUNT } from '../base/app';
 import { CONFERENCE_JOINED } from '../base/conference';
 import {
@@ -14,11 +12,13 @@ import { updateSettings } from '../base/settings';
 import { playSound, registerSound, unregisterSound } from '../base/sounds';
 import { hideNotification, showNotification } from '../notifications';
 
+import { setNoAudioSignalNotificationUid } from './actions';
+import { NO_AUDIO_SIGNAL_SOUND_ID } from './constants';
+import { NO_AUDIO_SIGNAL_SOUND_FILE } from './sounds';
+
 MiddlewareRegistry.register(store => next => async action => {
     const result = next(action);
-    const { dispatch, getState } = store;
+    const { dispatch } = store;
-    const { conference } = action;
-    let confAudioInputState;
 
     switch (action.type) {
     case APP_WILL_MOUNT:
@@ -27,93 +27,109 @@ MiddlewareRegistry.register(store => next => async action => {
     case APP_WILL_UNMOUNT:
         dispatch(unregisterSound(NO_AUDIO_SIGNAL_SOUND_ID));
         break;
-
+    case CONFERENCE_JOINED:
-    case CONFERENCE_JOINED: {
+        _handleNoAudioSignalNotification(store, action);
-        conference.on(JitsiConferenceEvents.AUDIO_INPUT_STATE_CHANGE, hasAudioInput => {
-            const { noAudioSignalNotificationUid } = getState()['features/no-audio-signal'];
-
-            confAudioInputState = hasAudioInput;
-
-            // In case the notification is displayed but the conference detected audio input signal we hide it.
-            if (noAudioSignalNotificationUid && hasAudioInput) {
-                dispatch(hideNotification(noAudioSignalNotificationUid));
-                dispatch(setNoAudioSignalNotificationUid());
-            }
-        });
-        conference.on(JitsiConferenceEvents.NO_AUDIO_INPUT, async () => {
-            const { noSrcDataNotificationUid } = getState()['features/base/no-src-data'];
-
-            // In case the 'no data detected from source' notification was already shown, we prevent the
-            // no audio signal notification as it's redundant i.e. it's clear that the users microphone is
-            // muted from system settings.
-            if (noSrcDataNotificationUid) {
-                return;
-            }
-
-            // Force the flag to false in case AUDIO_INPUT_STATE_CHANGE is received after the notification is displayed,
-            // possibly preventing the notification from displaying because of an outdated state.
-            confAudioInputState = false;
-
-
-            const activeDevice = await JitsiMeetJS.getActiveAudioDevice();
-
-            if (confAudioInputState) {
-                return;
-            }
-
-            // In case there is a previous notification displayed just hide it.
-            const { noAudioSignalNotificationUid } = getState()['features/no-audio-signal'];
-
-            if (noAudioSignalNotificationUid) {
-                dispatch(hideNotification(noAudioSignalNotificationUid));
-                dispatch(setNoAudioSignalNotificationUid());
-            }
-
-
-            let descriptionKey = 'toolbar.noAudioSignalDesc';
-            let customActionNameKey;
-            let customActionHandler;
-
-            // In case the detector picked up a device show a notification with a device suggestion
-            if (activeDevice.deviceLabel !== '') {
-                descriptionKey = 'toolbar.noAudioSignalDescSuggestion';
-
-                // Preferably the label should be passed as an argument paired with a i18next string, however
-                // at the point of the implementation the showNotification function only supports doing that for
-                // the description.
-                // TODO Add support for arguments to showNotification title and customAction strings.
-                customActionNameKey = `Use ${formatDeviceLabel(activeDevice.deviceLabel)}`;
-                customActionHandler = () => {
-                    // Select device callback
-                    dispatch(
-                            updateSettings({
-                                userSelectedMicDeviceId: activeDevice.deviceId,
-                                userSelectedMicDeviceLabel: activeDevice.deviceLabel
-                            })
-                    );
-
-                    dispatch(setAudioInputDevice(activeDevice.deviceId));
-                };
-            }
-
-            const notification = showNotification({
-                titleKey: 'toolbar.noAudioSignalTitle',
-                descriptionKey,
-                customActionNameKey,
-                customActionHandler
-            });
-
-            dispatch(notification);
-
-            dispatch(playSound(NO_AUDIO_SIGNAL_SOUND_ID));
-
-            // Store the current notification uid so we can check for this state and hide it in case
-            // a new track was added, thus changing the context of the notification
-            dispatch(setNoAudioSignalNotificationUid(notification.uid));
-        });
         break;
     }
-    }
 
     return result;
 });
+
+/**
+ * Handles the logic of displaying the no audio input detected notification as well as finding a valid device on the
+ * system.
+ *
+ * @param {Store} store - The redux store in which the specified action is being dispatched.
+ * @param {Action} action - The redux action {@code CONFERENCE_JOINED} which is being dispatched in the specified redux
+ * store.
+ * @private
+ * @returns {void}
+ */
+async function _handleNoAudioSignalNotification({ dispatch, getState }, action) {
+
+    const { conference } = action;
+    let confAudioInputState;
+
+    conference.on(JitsiConferenceEvents.AUDIO_INPUT_STATE_CHANGE, hasAudioInput => {
+        const { noAudioSignalNotificationUid } = getState()['features/no-audio-signal'];
+
+        confAudioInputState = hasAudioInput;
+
+        // In case the notification is displayed but the conference detected audio input signal we hide it.
+        if (noAudioSignalNotificationUid && hasAudioInput) {
+            dispatch(hideNotification(noAudioSignalNotificationUid));
+            dispatch(setNoAudioSignalNotificationUid());
+        }
+    });
+    conference.on(JitsiConferenceEvents.NO_AUDIO_INPUT, async () => {
+        const { noSrcDataNotificationUid } = getState()['features/base/no-src-data'];
+
+        // In case the 'no data detected from source' notification was already shown, we prevent the
+        // no audio signal notification as it's redundant i.e. it's clear that the users microphone is
+        // muted from system settings.
+        if (noSrcDataNotificationUid) {
+            return;
+        }
+
+        // Force the flag to false in case AUDIO_INPUT_STATE_CHANGE is received after the notification is displayed,
+        // possibly preventing the notification from displaying because of an outdated state.
+        confAudioInputState = false;
+
+
+        const activeDevice = await JitsiMeetJS.getActiveAudioDevice();
+
+        if (confAudioInputState) {
+            return;
+        }
+
+        // In case there is a previous notification displayed just hide it.
+        const { noAudioSignalNotificationUid } = getState()['features/no-audio-signal'];
+
+        if (noAudioSignalNotificationUid) {
+            dispatch(hideNotification(noAudioSignalNotificationUid));
+            dispatch(setNoAudioSignalNotificationUid());
+        }
+
+
+        let descriptionKey = 'toolbar.noAudioSignalDesc';
+        let customActionNameKey;
+        let customActionHandler;
+
+        // In case the detector picked up a device show a notification with a device suggestion
+        if (activeDevice.deviceLabel !== '') {
+            descriptionKey = 'toolbar.noAudioSignalDescSuggestion';
+
+            // Preferably the label should be passed as an argument paired with a i18next string, however
+            // at the point of the implementation the showNotification function only supports doing that for
+            // the description.
+            // TODO Add support for arguments to showNotification title and customAction strings.
+            customActionNameKey = `Use ${formatDeviceLabel(activeDevice.deviceLabel)}`;
+            customActionHandler = () => {
+                // Select device callback
+                dispatch(
+                        updateSettings({
+                            userSelectedMicDeviceId: activeDevice.deviceId,
+                            userSelectedMicDeviceLabel: activeDevice.deviceLabel
+                        })
+                );
+
+                dispatch(setAudioInputDevice(activeDevice.deviceId));
+            };
+        }
+
+        const notification = showNotification({
+            titleKey: 'toolbar.noAudioSignalTitle',
+            descriptionKey,
+            customActionNameKey,
+            customActionHandler
+        });
+
+        dispatch(notification);
+
+        dispatch(playSound(NO_AUDIO_SIGNAL_SOUND_ID));
+
+        // Store the current notification uid so we can check for this state and hide it in case
+        // a new track was added, thus changing the context of the notification
+        dispatch(setNoAudioSignalNotificationUid(notification.uid));
+    });
+}