Skip to content

Conversation

@yeshwanth-nagaswamy
Copy link
Contributor

@yeshwanth-nagaswamy yeshwanth-nagaswamy requested a review from a team as a code owner November 5, 2025 15:45
@yeshwanth-nagaswamy yeshwanth-nagaswamy force-pushed the feature/RDKEMW-100107 branch 3 times, most recently from 4e44852 to 68c03a9 Compare November 10, 2025 18:19

IARM_BUS_Lock(lock);
int volume = 0;
float m_volumeLevel = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is local variable u can remove m_ prefix to avoid confusion

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes an issue where the speaker volume level does not persist after a device reboot. The solution removes the static cached variable m_volumeLevel and replaces it with runtime calls to dsGetAudioLevel() to retrieve the current audio level from the underlying system.

  • Removed static variable m_volumeLevel that was caching volume level
  • Modified _dsSetAudioDucking() to call dsGetAudioLevel() to get current volume instead of using cached value
  • Updated setAudioDuckingAudioLevel() to retrieve volume dynamically when ducking is not in progress

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


ret = dsGetAudioLevel (param->handle, &volumeLevel);
if (ret != dsERR_NONE) {
INT_ERROR("%s failed dsGetAudioLevel\n",__FUNCTION__);
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If dsGetAudioLevel() fails, volumeLevel remains 0 and the function continues execution. This could result in incorrect volume calculations (e.g., setting volume to 0 during ducking). Consider either returning early on failure or using a fallback value to prevent potential volume loss.

Suggestion:

ret = dsGetAudioLevel (param->handle, &volumeLevel);
if (ret != dsERR_NONE) {
    INT_ERROR("%s failed dsGetAudioLevel\n",__FUNCTION__);
    IARM_BUS_Unlock(lock);
    return IARM_RESULT_INVALID_STATE;
}
Suggested change
INT_ERROR("%s failed dsGetAudioLevel\n",__FUNCTION__);
INT_ERROR("%s failed dsGetAudioLevel\n",__FUNCTION__);
IARM_BUS_Unlock(lock);
return IARM_RESULT_INVALID_STATE;

Copilot uses AI. Check for mistakes.
Comment on lines +3197 to 3202
INT_ERROR("%s dsGetAudioLevel\n",__FUNCTION__);
}
INT_DEBUG("%s: audio level %f\n", __FUNCTION__, volume);
}
if (dsSetAudioLevelFunc == 0)
{
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If dsGetAudioLevel() fails, volume remains 0 but the function continues to set this value via dsSetAudioLevelFunc() at line 3220. This could incorrectly set the audio level to 0. Consider returning an error when dsGetAudioLevel() fails to prevent incorrect volume settings.

Suggestion:

dsError_t ret = dsGetAudioLevel (handle, &volume);
if (ret != dsERR_NONE) {
    INT_ERROR("%s failed dsGetAudioLevel\n",__FUNCTION__);
    return IARM_RESULT_INVALID_STATE;
}
Suggested change
INT_ERROR("%s dsGetAudioLevel\n",__FUNCTION__);
}
INT_DEBUG("%s: audio level %f\n", __FUNCTION__, volume);
}
if (dsSetAudioLevelFunc == 0)
{
INT_ERROR("%s failed dsGetAudioLevel\n",__FUNCTION__);
return IARM_RESULT_INVALID_STATE;
}
INT_DEBUG("%s: audio level %f\n", __FUNCTION__, volume);
}
if (dsSetAudioLevelFunc == 0)

Copilot uses AI. Check for mistakes.
@apatel859 apatel859 merged commit c395cff into develop Nov 14, 2025
8 of 9 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants