Skip to content
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

Add AudioType Support #13

Merged
merged 2 commits into from
Nov 21, 2017
Merged

Add AudioType Support #13

merged 2 commits into from
Nov 21, 2017

Conversation

electrostat
Copy link
Contributor

Adds audioType support with a chain of responsibility of:

speaker > bluetooth > headphones > unknown

- add AudioTypeResolver
- add specific audio types
- add AudioTypeChain
- integrate into MapboxTelemetry
@@ -0,0 +1,16 @@
package com.mapbox.services.android.telemetry.audio;

public class AudioTypeChain {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this public?


public class AudioTypeChain {

public AudioTypeResolver setup() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here ☝️


import android.content.Context;

public interface AudioTypeResolver {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this public?

import android.content.Context;
import android.media.AudioManager;

public class HeadphonesAudioType implements AudioTypeResolver {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this public?

import android.content.Context;
import android.media.AudioManager;

public class SpeakerAudioType implements AudioTypeResolver {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this public?


import android.content.Context;

public class UnknownAudioType implements AudioTypeResolver {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this public?

@@ -277,4 +280,11 @@ public void onServiceDisconnected(ComponentName className) {
serviceBound = false;
}
};

public String obtainAudioType() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Where are we using obtainAudioType()? MapboxTelemetry shouldn't expose this functionality. obtainAudioType() should live in another class in which has more context of what it's doing. Events?

- create NavigationUtils
- return audiotype there to dev
- refactor audio classes out of their own directory, for later refactoring (likely into own Navigation directory)
@electrostat electrostat merged commit 55e1a84 into master Nov 21, 2017
@electrostat electrostat deleted the aa-audio-type-support branch November 21, 2017 15:11
electrostat added a commit that referenced this pull request Dec 13, 2017
* Add AudioType Support
* NavigationUtils + Refactor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants