Skip to content
This repository has been archived by the owner on Feb 2, 2021. It is now read-only.

Don't duplicate the messages from logcat #544

Merged
merged 1 commit into from
Nov 26, 2015

Conversation

Fatme
Copy link
Contributor

@Fatme Fatme commented Nov 24, 2015

@justcodebuilduser
Copy link

❤️

@teobugslayer
Copy link
Contributor

👍

@@ -10,7 +12,7 @@ export class LogcatHelper implements Mobile.ILogcatHelper {
private $staticConfig: Config.IStaticConfig) { }

public start(deviceIdentifier: string): void {
if(deviceIdentifier) {
if (deviceIdentifier && !this.isStarted) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this check should be per deviceIdentifier or we'll always have logs only for the first connected device (this will break Proton) :)

@Fatme Fatme force-pushed the fatme/fix-double-logging-on-android branch from 65bc4e1 to 1a38a63 Compare November 24, 2015 12:29
@justcodebuilduser
Copy link

❤️

@rosen-vladimirov
Copy link
Collaborator

Maybe I'm not correct, but what will happen in the following scenario:

  1. We have long living process (Proton) and we attach device with identifier A.
  2. We call logcatHelper for this device.
  3. A is added to mapDeviceToLoggingStarted dictionary.
  4. After some time the device is disconnected. A remains in the dictionary. adb process for logcat is dead.
  5. A is connected again.
  6. LogcatHelper is called for A, but as A is already in mapDeviceToLoggingStarted dictionary, we do not start new process for adb logcat.
  7. We do not receive logs for A

Maybe a better solution is to add new property to IDevice that indicates if logging is started and it will call LogcatHelper internally. As when new device is attached, I believe we create new instance of IDevice, so the above scenario could work fine there. Of course there could be various other solutions :)

@Fatme Fatme force-pushed the fatme/fix-double-logging-on-android branch from 1a38a63 to cffe2f8 Compare November 24, 2015 13:45
@justcodebuilduser
Copy link

❤️

@Fatme
Copy link
Contributor Author

Fatme commented Nov 24, 2015

If some device is disconnected, logcat close event will be raised. We'll execute this logic - https://github.com/telerik/mobile-cli-lib/pull/544/files#diff-618ae34084a745345392b83e2a7b243eR29 so the next time the device is connected, we will attach again on logcat data event and will receive logs from a device.

@rosen-vladimirov
Copy link
Collaborator

great, 👍

Fatme pushed a commit that referenced this pull request Nov 26, 2015
@Fatme Fatme merged commit e59cd56 into release Nov 26, 2015
@Fatme Fatme deleted the fatme/fix-double-logging-on-android branch November 26, 2015 14:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants