Fix for review feedback

Author: SteveMicroNova

amplipi/ctrl.py

@@ -850,14 +850,16 @@ def set_vol():
               vol, vol_f, vol_f_delta, vol_min, or vol_max.
           """
           # Field precedence: vol (db) > vol_delta > vol (float)
-          # vol (db) is first in precedence yet last in the stack to cover the default case of a None volume change, but when it does have a value it overrides the other options
+          # vol (db) is first in precedence yet last in the stack to cover the default case of no volume change
           if update.vol_delta_f is not None and update.vol is None:
             true_vol_f = zone.vol_f + zone.vol_f_overflow
             expected_vol_total = update.vol_delta_f + true_vol_f
             vol_f_new = utils.clamp(expected_vol_total, models.MIN_VOL_F, models.MAX_VOL_F)
 
             vol_db = utils.vol_float_to_db(vol_f_new, zone.vol_min, zone.vol_max)
-            zone.vol_f_overflow = 0 if models.MIN_VOL_F < expected_vol_total and expected_vol_total < models.MAX_VOL_F else utils.clamp((expected_vol_total - vol_f_new), models.MIN_VOL_F_OVERFLOW, models.MAX_VOL_F_OVERFLOW)  # Clamp the remaining delta to be between -1 and 1
+            zone.vol_f_overflow = 0 if models.MIN_VOL_F < expected_vol_total and expected_vol_total < models.MAX_VOL_F \
+                else utils.clamp((expected_vol_total - vol_f_new), models.MIN_VOL_F_OVERFLOW, models.MAX_VOL_F_OVERFLOW)
+            # Clamp the remaining delta to be between -1 and 1
 
           elif update.vol_f is not None and update.vol is None:
             clamp_vol_f = utils.clamp(vol_f, 0, 1)
@@ -874,8 +876,8 @@ def set_vol():
           else:
             raise Exception('unable to update zone volume')
 
-          # If the change made vol f be between the min and max, delete the overflow
-          # This is useful so that you can click wherever you want on the volume bar and expect it to end up there without rubberbanding back to whatever vol_f + vol_f_overflow value you'd otherwise be at
+          # Reset the overflow when vol_f goes in bounds, there is no longer an overflow
+          # Avoids reporting spurious volume oscillations
           zone.vol_f_overflow = 0 if vol_f_new != models.MIN_VOL_F and vol_f_new != models.MAX_VOL_F else zone.vol_f_overflow
 
         # To avoid potential unwanted loud output:

web/src/App.jsx

@@ -50,11 +50,12 @@ export const useStatusStore = create((set, get) => ({
                 applyPlayerVol(vol, zones, sourceId, (zone_id, new_vol) => {
                     for (const i in s.status.zones) {
                         if (s.status.zones[i].id === zone_id) {
-                            let true_vol = Math.round((new_vol + s.status.zones[i].vol_f_overflow) * 100) / 100;
-                            let clamped = Math.min(Math.max(true_vol, 0), 1);
+                            // Calculate out future vol_f and vol_f_overflow to match expected future API polled state
+                            let combined_vol = new_vol + s.status.zones[i].vol_f_overflow;
+                            let new_vol_f = Math.min(Math.max(combined_vol, 0), 1);
 
-                            s.status.zones[i].vol_f = clamped;
-                            s.status.zones[i].vol_f_overflow = true_vol - clamped;
+                            s.status.zones[i].vol_f = new_vol_f;
+                            s.status.zones[i].vol_f_overflow = combined_vol - new_vol_f;
                         }
                     }
                 });
@@ -72,7 +73,7 @@ export const useStatusStore = create((set, get) => ({
                     }
                 }
 
-                // Also update groups that consist entirely of affected zones
+                // Mute groups if they are now completely muted
                 for (const g of s.status.groups) {
                     if (g.zones.every(zid => affectedZones.includes(zid))) {
                         g.mute = mute;
@@ -174,7 +175,9 @@ export const useStatusStore = create((set, get) => ({
                 const g = s.status.groups.filter((g) => g.id === groupId)[0];
                 for (const i of g.zones) {
                     s.skipUpdate = true;
-                    s.status.zones[i].vol_f = new_vol + s.status.zones[i].vol_f_overflow;
+                    // vol_f_overflow is set to 0 whenever vol_f is between 0 and 1, groups authoritatively set the volume so we reflect that here too
+                    s.status.zones[i].vol_f_overflow = 0;
+                    s.status.zones[i].vol_f = new_vol;
                 }
 
                 updateGroupVols(s);
@@ -208,6 +211,7 @@ export const useStatusStore = create((set, get) => ({
 const updateGroupVols = (s) => {
     s.status.groups.forEach((g) => {
         if (g.zones.length > 1) {
+            // Combine vol_f with vol_f_overflow to ensure the group volume slider moves at the same relative speed even when a zone overflows
             const vols = g.zones.map((id) => s.status.zones[id].vol_f + s.status.zones[id].vol_f_overflow);
             let calculated_vol = Math.min(...vols) * 0.5 + Math.max(...vols) * 0.5;
             g.vol_f = calculated_vol;