-
-
Notifications
You must be signed in to change notification settings - Fork 980
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
Turn screen on Plugin #7031
Turn screen on Plugin #7031
Conversation
Please rename OsmAnd-Plugin-TurnScreenOn |
private RadioGroupWrapper osmandVersionRadioGroup; | ||
private FrameLayout btnOpenOsmand; | ||
|
||
private LayoutInflater inflater; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use local variable
|
||
if (!settings.isProgramOpenedEarlier()) { | ||
settings.setOpened(); | ||
isReturnedFromAnotherActivity = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this var needed? Is it possible to move code from onResume to onCreate?
settings.setOpened(); | ||
isReturnedFromAnotherActivity = true; | ||
startPluginDescriptionActivity(PluginDescriptionActivity.MODE_FIRST_OPEN); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
formatting
if (settings.hasAvailableOsmandVersions()) { | ||
try { | ||
if (!isReturnedFromAnotherActivity) { | ||
Log.d("ttpl2", "no returned"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove such logs from PR.
setContentView(R.layout.activity_main); | ||
|
||
app = (TurnScreenApp) getApplicationContext(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra space
|
||
public void registerForVoiceRouterMessages() { | ||
try { | ||
Log.d("ttpl", "try add a listener"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
drop dev logs
isRegistered = false; | ||
} | ||
} catch (RemoteException e) { | ||
e.printStackTrace(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do not use e.printStackTrace() (even if you copy-pasted it)
refreshUI(); | ||
} | ||
} catch (Exception e) { | ||
Log.d("ttpl2", "take an exception"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what exception?
@@ -0,0 +1,8 @@ | |||
package net.osmand.turnScreenOn.listener; | |||
|
|||
public interface MessageSender { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like abstract, but used for single purpose. Needs to be renamed?
import android.util.SparseArray; | ||
import android.widget.RadioButton; | ||
|
||
public class RadioGroupWrapper { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Purpose is unclear.
voiceMessageListeners.remove(voiceMessageListener); | ||
} | ||
|
||
public void notifyOnVoiceMessage() { | ||
Log.d("ttpl", "notify listeners: "); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Log.d should be deleted - only for dev and we use commons logging for prod
@@ -1897,6 +1899,33 @@ public void unregisterFromUpdates(long id) { | |||
navUpdateCallbacks.remove(id); | |||
} | |||
|
|||
private Map<Long, VoiceRouter.VoiceMessageListener> voiceRouterMessageCallbacks= new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fields in top of the class
int i = 0; | ||
for (VoiceMessageListener lnt : voiceMessageListeners.keySet()) { | ||
Log.d("ttpl", "listner " + ++i); | ||
lnt.onVoiceMessage(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Paramater which voice message should be present
@@ -8,6 +8,7 @@ | |||
import java.util.concurrent.ConcurrentHashMap; | |||
|
|||
import net.osmand.Location; | |||
import net.osmand.aidl.OsmandAidlService; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this class shouldn't be present here
@@ -27,6 +28,7 @@ | |||
import alice.tuprolog.Term; | |||
import android.media.AudioManager; | |||
import android.media.SoundPool; | |||
import android.util.Log; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
class should be deleted
@@ -27,6 +28,7 @@ | |||
import alice.tuprolog.Term; | |||
import android.media.AudioManager; | |||
import android.media.SoundPool; | |||
import android.util.Log; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use weakreference for listeners
public final CommonPreference<Integer> WAKE_ON_VOICE_TIME_INT = new IntPreference("wake_on_voice_time_int", 0).makeProfile(); | ||
|
||
{ | ||
// 0 means never |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// 0 means never?
|
||
{ | ||
// 0 means never | ||
WAKE_ON_VOICE_SENSOR.setModeDefaultValue(ApplicationMode.CAR, false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete theses blocks
LOG.debug("switch on sensor"); | ||
mSensorManager.registerListener(this, mProximity, SensorManager.SENSOR_DELAY_NORMAL); | ||
} else { | ||
LOG.debug("switch off sensor"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
debug?
} | ||
play(null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double play?
} | ||
|
||
public void switchOnSensor() { | ||
LOG.debug("switch on sensor"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete debug
voiceMessageListener = new VoiceRouter.VoiceMessageListener() { | ||
@Override | ||
public void onVoiceMessage() { | ||
LOG.debug("onVoiceMessage"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
debug?
@@ -705,4 +706,7 @@ interface IOsmAndAidlInterface { | |||
boolean areOsmandSettingsCustomized(in OsmandSettingsInfoParams params); | |||
|
|||
boolean setCustomization(in CustomizationInfoParams params); | |||
long registerForVoiceRouterMessages(in ANavigationVoiceRouterMessageParams params, IOsmAndAidlCallback callback); | |||
|
|||
boolean changeMapActivityKeyguardFlags(in boolean enable); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add comments
…_screen_on_plugin
}; | ||
|
||
setVoiceRouterListener(true); | ||
setSensor(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Start sensor only if turned on in settings
public void onSensorChanged(SensorEvent event) { | ||
if (event.sensor.getType() == Sensor.TYPE_PROXIMITY) { | ||
if (event.values[0] >= -SENSOR_SENSITIVITY && event.values[0] <= SENSOR_SENSITIVITY) { | ||
if (isSensorEnabled()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Such check must be at top of the method
} | ||
}; | ||
|
||
setVoiceRouterListener(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why it is not the same - f9cd73d#diff-33da24854a630e0918b813683c839fbbL79
move sensor state verification
Now it activates only if enable in the settings and the screen is off while navigation. Remove router checkbox from preferences
Now it activates only if enable in the settings and the screen is off while navigation. Remove router checkbox from preferences
fix changeKeyguardFlags (MapActivity) multithreading problem
No description provided.