-
-
Notifications
You must be signed in to change notification settings - Fork 30
Volume Delta Expansion #1062
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: main
Are you sure you want to change the base?
Volume Delta Expansion #1062
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 |
|---|---|---|
|
|
@@ -605,20 +605,44 @@ def test_patch_zones_vol_delta(client): | |
| # check that each update worked as expected | ||
| for z in jrv['zones']: | ||
| if z['id'] in range(6): | ||
| assert z['vol_f'] - (zones[z['id']]['vol_f'] + 0.1) < 0.0001 | ||
| assert z['vol_f'] - (zones[z['id']]['vol_f'] + 0.1) < 0.0001 and z["vol_f_overflow"] == 0 | ||
|
|
||
| # test oversized deltas | ||
| rv = client.patch('/api/zones', json={'zones': [z['id'] for z in zones], 'update': {'vol_delta_f': -10.0}}) | ||
| # test overflowing deltas | ||
| rv = client.patch('/api/zones', json={'zones': [z['id'] for z in zones], 'update': {'vol_delta_f': -1.0}}) | ||
| assert rv.status_code == HTTPStatus.OK | ||
| jrv = rv.json() | ||
| assert len(jrv['zones']) >= 6 | ||
| # check that each update worked as expected | ||
| for z in jrv['zones']: | ||
| if z['id'] in range(6): | ||
| assert z['vol_f'] == amplipi.models.MIN_VOL_F | ||
| assert z["vol_f_overflow"] == zones[z['id']]['vol_f'] + 0.1 - 1 | ||
|
|
||
| # test oversized overflowing deltas | ||
| rv = client.patch('/api/zones', json={'zones': [z['id'] for z in zones], 'update': {'vol_delta_f': 10.0}}) | ||
| assert rv.status_code == HTTPStatus.OK | ||
| jrv = rv.json() | ||
| assert len(jrv['zones']) >= 6 | ||
| # check that each update worked as expected | ||
| for z in jrv['zones']: | ||
| if z['id'] in range(6): | ||
| assert z['vol_f'] == amplipi.models.MAX_VOL_F | ||
| assert z["vol_f_overflow"] == amplipi.models.MAX_VOL_F_OVERFLOW | ||
|
|
||
| # test overflow reset | ||
| mid_vol_f = (amplipi.models.MIN_VOL_F + amplipi.models.MAX_VOL_F) / 2 | ||
| rv = client.patch('/api/zones', json={'zones': [z['id'] for z in zones], 'update': {'vol_f': mid_vol_f}}) | ||
| assert rv.status_code == HTTPStatus.OK | ||
| jrv = rv.json() | ||
| assert len(jrv['zones']) >= 6 | ||
| # check that each update worked as expected | ||
| for z in jrv['zones']: | ||
| if z['id'] in range(6): | ||
| assert z['vol_f'] == mid_vol_f | ||
| assert z["vol_f_overflow"] == 0 | ||
|
Comment on lines
+608
to
+642
Contributor
Author
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. Changed this test to account for valid overflows, oversized overflows, and overflow resets |
||
|
|
||
| # test precedence | ||
| rv = client.patch('/api/zones', json={'zones': [z['id'] for z in zones], 'update': {'vol_delta_f': 10.0, "vol": amplipi.models.MIN_VOL_DB}}) | ||
| rv = client.patch('/api/zones', json={'zones': [z['id'] for z in zones], 'update': {'vol_delta_f': 1.0, "vol": amplipi.models.MIN_VOL_DB}}) | ||
| assert rv.status_code == HTTPStatus.OK | ||
| jrv = rv.json() | ||
| assert len(jrv['zones']) >= 6 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,7 @@ const getPlayerVol = (sourceId, zones) => { | |
| let n = 0; | ||
| for (const i of getSourceZones(sourceId, zones)) { | ||
| n += 1; | ||
| vol += i.vol_f; | ||
| vol += i.vol_f + i.vol_f_overflow; // Add buffer to retain proper relative space when doing an action that would un-overload the slider | ||
| } | ||
|
|
||
| const avg = vol / n; | ||
|
|
@@ -27,22 +27,23 @@ export const applyPlayerVol = (vol, zones, sourceId, apply) => { | |
| let delta = vol - getPlayerVol(sourceId, zones); | ||
|
|
||
| for (let i of getSourceZones(sourceId, zones)) { | ||
| let set_pt = Math.max(0, Math.min(1, i.vol_f + delta)); | ||
| apply(i.id, set_pt); | ||
| apply(i.id, i.vol_f + delta); | ||
| } | ||
| }; | ||
|
|
||
| // cumulativeDelta reflects the amount of movement that the | ||
| let cumulativeDelta = 0; | ||
| // cumulativeDelta reflects the amount of movement that the volume bar has had that has gone unreflected in the backend | ||
| let cumulativeDelta = 0.0; | ||
| let previous_delta = null; | ||
| let sendingPacketCount = 0; | ||
|
|
||
| // main volume slider on player and volume slider on player card | ||
| const CardVolumeSlider = ({ sourceId }) => { | ||
| const zones = useStatusStore((s) => s.status.zones); | ||
| const setZonesVol = useStatusStore((s) => s.setZonesVol); | ||
| const setZonesMute = useStatusStore((s) => s.setZonesMute); | ||
| const setSystemState = useStatusStore((s) => s.setSystemState); | ||
|
|
||
| // needed to ensure that polling doesn't cause the delta volume to be made inacurrate during volume slider interactions | ||
| // needed to ensure that polling doesn't cause the delta volume to be made inaccurate during volume slider interactions | ||
| const skipNextUpdate = useStatusStore((s) => s.skipNextUpdate); | ||
|
|
||
| const value = getPlayerVol(sourceId, zones); | ||
|
|
@@ -52,32 +53,42 @@ const CardVolumeSlider = ({ sourceId }) => { | |
| setZonesMute(false, zones, sourceId); | ||
| }; | ||
|
|
||
| function setPlayerVol(vol, val) { | ||
| cumulativeDelta += vol - val; | ||
|
|
||
| if(sendingPacketCount <= 0){ | ||
| function setPlayerVol(force = false) { | ||
| if(sendingPacketCount <= 0 || force){ // Sometimes slide and release interactions can send the same request twice, this check helps prevent that from doubling the intended change | ||
| sendingPacketCount += 1; | ||
|
|
||
| const delta = cumulativeDelta; | ||
|
|
||
| fetch("/api/zones", { | ||
| method: "PATCH", | ||
| headers: { | ||
| "Content-type": "application/json", | ||
| }, | ||
| body: JSON.stringify({ | ||
| zones: getSourceZones(sourceId, zones).map((z) => z.id), | ||
| update: { vol_delta_f: cumulativeDelta, mute: false }, | ||
| }), | ||
| }).then(() => { | ||
| // NOTE: This used to just set cumulativeDelta to 0 | ||
| // that would skip all accumulated delta from fetch start to backend response time | ||
| // causing jittering issues | ||
| cumulativeDelta -= delta; | ||
| if(previous_delta !== delta){ | ||
| previous_delta = delta; | ||
|
|
||
| fetch("/api/zones", { | ||
| method: "PATCH", | ||
| headers: { | ||
| "Content-type": "application/json", | ||
| }, | ||
| body: JSON.stringify({ | ||
| zones: getSourceZones(sourceId, zones).map((z) => z.id), | ||
| update: { vol_delta_f: delta, mute: false }, | ||
| }), | ||
| }).then(() => { | ||
| // NOTE: This used to just set cumulativeDelta to 0 | ||
| // that would skip all accumulated delta from fetch start to backend response time | ||
| // causing jittering issues | ||
| if(force){ | ||
| cumulativeDelta = 0.0; // If you force push a change, do not store any unrecognized changes | ||
| } else { | ||
| cumulativeDelta -= delta; // If this is a normal change, there is likely a change coming up and setting to zero would lose part of that upcoming delta | ||
| } | ||
| sendingPacketCount -= 1; | ||
| // In many similar requests we instantly consume the response by doing something like this: | ||
| // if(res.ok){res.json().then(s=> setSystemState(s));} | ||
| // This cannot be done here due to how rapid fire the requests can be when sliding the slider rather than just tapping it | ||
| }); | ||
| } else { | ||
| sendingPacketCount -= 1; | ||
| }); | ||
| } | ||
| } | ||
| }; | ||
| } | ||
|
Comment on lines
+56
to
+91
Contributor
Author
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. This is the slider logic that handles slides, taps, and delta measurements for the frontend |
||
|
|
||
| const mute = getSourceZones(sourceId, zones) | ||
| .map((z) => z.mute) | ||
|
|
@@ -95,6 +106,8 @@ const CardVolumeSlider = ({ sourceId }) => { | |
| zones: getSourceZones(sourceId, zones).map((z) => z.id), | ||
| update: { mute: mute }, | ||
| }), | ||
| }).then(res => { | ||
| if(res.ok){res.json().then(s => setSystemState(s));} | ||
| }); | ||
| }; | ||
|
|
||
|
|
@@ -107,9 +120,9 @@ const CardVolumeSlider = ({ sourceId }) => { | |
| setVol={(val, force) => { | ||
| // Cannot use value directly as that changes during the request when setValue() is called | ||
| // Cannot call setValue() as a .then() after the request as that causes the ui to feel unresponsive and choppy | ||
| let current_val = value; | ||
| setPlayerVol(val, current_val); | ||
| cumulativeDelta += Math.round((val - value) * 1000) / 1000; | ||
| setValue(val); | ||
| setPlayerVol(force); | ||
| skipNextUpdate(); | ||
| }} | ||
| disabled={getSourceZones(sourceId, zones) == 0} | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.