Skip to content
This repository has been archived by the owner on Jan 16, 2024. It is now read-only.

Alerts volume reset to 100 instead of reading from database on startup #2068

Open
2 of 8 tasks
brett-lynnes opened this issue Jan 5, 2023 · 3 comments
Open
2 of 8 tasks

Comments

@brett-lynnes
Copy link

brett-lynnes commented Jan 5, 2023

IMPORTANT: Before you create an issue, please take a look at our Issue Reporting Guide.

Briefly summarize your issue:

AVS_ALERTS_VOLUME type ChannelVolumeInterfaces are always being loaded/started at 100, even if the state saved in the database (and online settings) have it at a lower volume.

Because the SpeakerManager object is created before the ChannelVolumeInterfaces are added to the speaker_map (code from DefaultClient.cpp in 2.6.0) the defaults are being set during initialization instead of the presetChannelDefaults (called during creation of SpeakerManager)


    auto speakerManager = manufactory->get<std::shared_ptr<SpeakerManagerInterface>>();
    if (!speakerManager) {
        ACSDK_ERROR(LX("createFailed").d("reason", "nullSpeakerManager"));
        return nullptr;
    }
    for (const auto& it : audioChannelVolumeInterfaces) {
        speakerManager->addChannelVolumeInterface(it);
    }

When the ChannelVolumeInterface is constructed it initializes the volumes during the method SpeakerManager::addChannelVolumeInterfaceIntoSpeakerMap(...)

this method gets a SpeakerSettings object, calls executeInitializeSpeakerSettings on it (sets it to 100), then calls updateContextManager() on that. unfortunately this only happens for AVS_SPEAKER_VOLUME and not AVS_ALERTS_VOLUME.

line 222(ish) of SpeakerManager.cpp

        // If we have one AVS_SPEAKER_VOLUME speaker, update the Context initially.
        if (ChannelVolumeInterface::Type::AVS_SPEAKER_VOLUME == type) {
            SpeakerInterface::SpeakerSettings settings;
            if (!executeGetSpeakerSettings(type, &settings) || !updateContextManager(type, settings)) {
                ACSDK_ERROR(LX(__func__).m("getSpeakerSettingsFailed or initialUpdateContextManagerFailed"));
            }
        }

later, code is called to check and set the AVS_SPEAKER_VOLUME explicitly to the settings volume, which has been updated in the context, but this doesnt happen for the alerts volume.

to fix this, the channelVolumeInterfaces must be added to the speaker_map before SpeakerManager is created, or the updateChannelSettings() should be called in initialization instead of construction.

What is the expected behavior?

Alerts volume settings should persist between reboots

What behavior are you observing?

system volume settings persists between reboots, but alerts volume does not.

Provide the steps to reproduce the issue, if applicable:

Easy reproduction: start the app. say "alexa set alarm volume to 1". test an alarm. shutdown and restart the app. now test another alarm; it will trigger at 100% volume.

Tell us about your environment:

What version of the AVS Device SDK are you using?

  <3.0.0>

Tell us what hardware you're using:

  • Desktop / Laptop
  • Raspberry Pi
  • Other - tell us more:

Tell us about your OS (Type & version):

  • Linux
  • MacOS
  • Raspbian Stretch
  • Raspbian Jessy
  • Other - tell us more:
@brett-lynnes
Copy link
Author

I was able to fix this on our devices by replacing line 224 of SpeakerManager.cpp

if (!executeInitializeSpeakerSettings(type)) {
            ACSDK_ERROR(LX("executeInitializeSpeakerSettings failed"));
        }

with this line

updateChannelSettings(type);

which will call eventually call executeInitializeSpeakerSettings(type) after updating correctly (since the update on line 206 never actually happened due to them not being in the map at that time).

@watsocao
Copy link

watsocao commented Jan 6, 2023

Hello @brett-lynnes,
Thank you for bringing this issue to our attention. A formal fix for this issue is being investigated internally.

@brett-lynnes
Copy link
Author

i also had to change the call to executeInitializeSpeakerSettings(...) in updateChannelSettings(...) to executeSetSpeakerSettings(...) since the "settings" in Initialize was not formed correctly at that point

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants