Skip to content

fix: GetMute package#41

Merged
blorp-goes-the-derp merged 2 commits intoobs-websocket-community-projects:developfrom
ChristopheCVB:fix/mute
Jun 11, 2021
Merged

fix: GetMute package#41
blorp-goes-the-derp merged 2 commits intoobs-websocket-community-projects:developfrom
ChristopheCVB:fix/mute

Conversation

@ChristopheCVB
Copy link
Copy Markdown
Member

No description provided.

@ChristopheCVB
Copy link
Copy Markdown
Member Author

@TinaTiel Can you merge this please?

@blorp-goes-the-derp
Copy link
Copy Markdown
Collaborator

blorp-goes-the-derp commented Jun 10, 2021

I'll take a look later tonight.
Could you merge in the recent pushes from the develop branch in the meantime?

Copy link
Copy Markdown
Collaborator

@blorp-goes-the-derp blorp-goes-the-derp left a comment

Choose a reason for hiding this comment

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

Looks good, only suggestion I have is to update the test to capture the getMute callback correctness.

I've done this privately, but don't have permission to push to your remote -- so if you want to use this you're welcome to:
ObsRemoteE2eObservationIT.java

  @Test
  void setVolumeAndMute() {

    obsShould("Set the volume to 50% (note, appears 67% due to log scaling; check % in advanced audio properties)");
    remote.setSourceVisibility(null, "media", true, loggingCallback);
    remote.setVolume(SOURCE_MEDIA, 0.50, loggingCallback);
    obsShould("Mute the volume");
    remote.setMute(SOURCE_MEDIA, true, loggingCallback);
    remote.getMute(SOURCE_MEDIA, capturingCallback);
    waitReasonably();
    assertThat(getPreviousResponseAs(GetMuteResponse.class).isMuted()).isTrue();

    obsShould("Unmute the volume");
    remote.setMute(SOURCE_MEDIA, false, loggingCallback);
    remote.getMute(SOURCE_MEDIA, capturingCallback);
    waitReasonably();
    assertThat(getPreviousResponseAs(GetMuteResponse.class).isMuted()).isFalse();

    obsShould("Set the volume to 100%");
    remote.setVolume(SOURCE_MEDIA, 1.00, loggingCallback);

  }

Unrelated -- I'll create a separate branch to cleanup these tests a bit after we merge in yours; I think the waitReasonably() method should be part of getPreviousResponseAs(...) since it always needs to happen. Plus, there's your well-timed feedback on the other PR 🤣

Copy link
Copy Markdown
Collaborator

@blorp-goes-the-derp blorp-goes-the-derp left a comment

Choose a reason for hiding this comment

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

Thanks! Great work

@blorp-goes-the-derp blorp-goes-the-derp merged commit 4dc94e1 into obs-websocket-community-projects:develop Jun 11, 2021
@ChristopheCVB ChristopheCVB deleted the fix/mute branch June 12, 2021 14:34
@blorp-goes-the-derp blorp-goes-the-derp mentioned this pull request Jun 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants