Frequency enforcement#2388
Conversation
|
Please merge the other PR first as this one shares a lot of common code |
|
Having medium and high enforcement makes sense to me so we can test this. Do we plan on adding a low enforcement? If not, why not have a boolean variable like shouldSetVolumeToMax that's easier to understand? |
HoangHongHieu
force-pushed
the
hieuhh/frequency_enforcement
branch
from
c31d074 to
d7579b4
Compare
Following is the app behavior when the feature is enabled or disabled:
- Enable:
The volume is set to maximum at the beginning of all frequency tests.
During the test, any changes to the volume or peripherals will cause the test to fail.
The test will fail if the required peripherals are missing.
- Disable:
A warning will be raised if the required peripherals are missing or volume is changed.
HoangHongHieu
force-pushed
the
hieuhh/frequency_enforcement
branch
from
d7579b4 to
131f34b
Compare
Thanks for reviewing, I didn't intend to add a low enforcement, so I updated the commit to use the boolean variable. The vibe code looked a bit messy and hard to read, so I refactored some of the functions in the frequency tests. |
| import java.util.TimerTask; | ||
| import java.util.Timer; | ||
|
|
||
| public class TestSupervisor extends AudioDeviceCallback { |
There was a problem hiding this comment.
Please add comments on why we need a test supervisor and why it's intended to be used. If we don't plan on having this for multiple tests, why not just subscribe to device changed callbacks within the test and check for volume changes in the test?
I personally don't like classes like this as it's not obvious what the class does from the naming. One thing we can do is maybe add a simple class that just checks whether the volume has changed. For checking if the devices have changed, we already have onAudioDevicesAdded and onAudioDevicesRemoved
| private void showMaxVolumeConfirmationDialog() { | ||
| new android.app.AlertDialog.Builder(this) | ||
| .setTitle("Volume Warning") | ||
| .setMessage("High enforcement mode will set the volume to MAXIMUM. This may be very loud. Do you want to proceed?") |
There was a problem hiding this comment.
Still mentions high enforcement
| }); | ||
|
|
||
| AudioManager audioManager = (AudioManager) getSystemService(Context.AUDIO_SERVICE); | ||
| mTestSupervisor = new TestSupervisor(audioManager, AudioManager.STREAM_MUSIC); |
There was a problem hiding this comment.
I noticed that this defaults to stream music. What if we use other types of streams?
2 participants
Frequency tests require a high volume to generate a clear frequency response from the speaker or microphone; however, the volume level is often left unset. Additionally, peripheral adjustments or volume changes during testing can lead to unexpected results. This ticket introduces an enforcement mode to automatically set the volume and monitor any changes during the test. Following is the app behavior when the feature is disabled or enabled:
The default enforcement mode is enabled, though users are free to change. This approach increases the application's flexibility.