Discussion:
[BitPim-devel] LGDM Phone patch
Joe Pham
2007-06-26 03:37:52 UTC
Permalink
I finally got some time to look at the patch and I have a few
suggestions:

1. Need to determine when a phone dropped off DM and needs re-enter
DM, vs when it failed to enter DM in the first place (which should be
ignored rather than retry).

2. Your wrappers for file ops need to be done per instance vs per
class basis. (Your scheme of using a class variable does not seem to
work anyway).

3. Inside your wrappers, just check if DM has been achieved, if not
then try it rather than catching for BrewAccessDeniedException.

-Joe Pham

_____________________________________________________________
Click to find local singles for dating, romance and fun
http://track.netzero.net/s/lc?u=http://tagline.untd.us/fc/Ioyw6ijm1uSd6H6iB6wFjd5P5l808R0S9HWrGyQbRBrT4hnj8UhyPi/
Nathan Hjelm
2007-06-26 07:52:28 UTC
Permalink
Post by Joe Pham
I finally got some time to look at the patch and I have a few
1. Need to determine when a phone dropped off DM and needs re-enter
DM, vs when it failed to enter DM in the first place (which should be
ignored rather than retry).
I am unsure a way exists to determine what the DM state of the phone
is except by trying a file operation and detecting a failure. I will
look into other means of detecting DM but I do not expect another
method will be found (LGD simply sets DM before any file operation
regardless of phone state).

A failure to enter DM should raise an exception (in DMv5).
Post by Joe Pham
2. Your wrappers for file ops need to be done per instance vs per
class basis. (Your scheme of using a class variable does not seem to
work anyway).
Can you explain what you mean by the class variable scheme not
working? I have tested the code with both a VX-8700 and VX-8600 (v3).

I am unclear on what you mean by per instance basis vs per class
basis? The code *should* already be working on a per instance basis.
Post by Joe Pham
3. Inside your wrappers, just check if DM has been achieved, if not
then try it rather than catching for BrewAccessDeniedException.
How you suggest is how I had previously handled entering DM. I
changed it 1) to avoid messing with DM when it is not needed (the
phone may already be in DM or may not need it for the particular
operation) and 2) to simply catch when the phone has left DM.


Thanks for the suggestions! I look forward to providing an improved
patch :)

-Nathan Hjelm
Joe Pham
2007-06-26 22:26:55 UTC
Permalink
Post by Nathan Hjelm
Post by Joe Pham
1. Need to determine when a phone dropped off DM and needs re-enter
DM, vs when it failed to enter DM in the first place (which should
be ignored rather than retry).
I am unsure a way exists to determine what the DM state of the
phone is except by trying a file operation and detecting a failure.
Let me rephrase: only retry if you successfully transitioned to DM,
else just ignore. Another approach is to mirror what LG does: try to
enter DM for every ops (for those phones that need it).
Post by Nathan Hjelm
Can you explain what you mean by the class variable scheme not
working?
# don't call setup code more than once
if self._DM_inited == 0:

-Joe Pham


_____________________________________________________________
Click for FHA loan, $0 lender fees, low rates & approvals nationwide
http://track.netzero.net/s/lc?u=http://tagline.untd.us/fc/Ioyw6ijldpwFjRVzDGhtIqplJ5cdCmdgzKSOuSQ7SbYRkG2PDOlitS/
Nathan Hjelm
2007-06-27 03:09:01 UTC
Permalink
Post by Joe Pham
Let me rephrase: only retry if you successfully transitioned to DM,
else just ignore. Another approach is to mirror what LG does: try to
enter DM for every ops (for those phones that need it).
I see what you mean. I corrected the code to raise the
com_brew.BrewAccessDeniedException if it fails to enter DM. This
implementation should be almost entirely transparent to any phones
that don't need/use DM. :)
Post by Joe Pham
Post by Nathan Hjelm
Can you explain what you mean by the class variable scheme not
working?
# don't call setup code more than once
Ah, I think I see what you mean. I changed how my LGDMPhone code is
initialized. Take a look and let me know if it is more to your
liking. :)

Also take a look my modifications to the sendbrewcommand code. With
this patch it will now throw com_brew.BrewAccessDeniedException if
the phone returns any sequence with \x4b as the first byte and \x1c
as the third. Seemed silly to check for access denied with each
possible command.
Joe Pham
2007-06-29 23:01:45 UTC
Permalink
I've made some changes to the LGDMPhone class and committed the
changes. Please take a look at the changes to see if they'd work for
you.

-Joe Pham

_____________________________________________________________
Turn your passion into a profession. Click here to find a film school near you.
http://track.netzero.net/s/lc?u=http://tagline.untd.us/fc/Ioyw6ijlSMOzpBp8XZUY7JR0zywXFMU6iNGfobYMzg4c8IWfk2mvqs/
Nathan Hjelm
2007-06-30 01:09:31 UTC
Permalink
Your changes look good for the most part. One thing that will not work is your hardcoding of the DM version. Verizon is adding DMv5 with each new firmware. By always attempting DMv5 and falling back on DMv4 these new firmwares can be supported without any change to Bitpim. Trying DMv5 has no apparent negative effect on non-DM phones and just returns a command not supported error (confirmed with a VX-8600v2 and VX-8300v2). I would suggest removing the hardcoding of the DM version (as it was in my patch). As it is written now the VX-8600v3 and VX-9900v2 are not supported (DMv5).

Also, operating on a timer should work fine but, again, trying the command and getting the access denied has no apparent negative effect.

-Nathan
Post by Joe Pham
I've made some changes to the LGDMPhone class and committed the
changes. Please take a look at the changes to see if they'd work for
you.
-Joe Pham
_____________________________________________________________
Turn your passion into a profession. Click here to find a film school near you.
http://track.netzero.net/s/lc?u=http://tagline.untd.us/fc/Ioyw6ijlSMOzpBp8XZUY7JR0zywXFMU6iNGfobYMzg4c8IWfk2mvqs/
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
BitPim-devel mailing list
https://lists.sourceforge.net/lists/listinfo/bitpim-devel
Joe Pham
2007-06-30 02:53:07 UTC
Permalink
Post by Nathan Hjelm
One thing that will not work is your hardcoding of the DM version.
It's not hardcoded, subclass can change it via "setDMversion()".
Post by Nathan Hjelm
operating on a timer should work fine
That was meant for those phone models that may get kicked out of DM
after some amount of time. Obviously, one has to determine, or
guess, how long that is.
Post by Nathan Hjelm
trying the command and getting the access denied has no apparent
negative effect.
Not sure what you meant, is that a good or bad thing?

-Joe Pham

_____________________________________________________________
Click to find local singles for dating, romance and fun
http://track.netzero.net/s/lc?u=http://tagline.untd.us/fc/Ioyw6ijm1uR4SC9PdQ0bHcerXz8EBT7s8o23pHXnGiM1SvOzrna54Q/
Nathan Hjelm
2007-06-30 19:17:06 UTC
Permalink
Post by Joe Pham
Post by Nathan Hjelm
One thing that will not work is your hardcoding of the DM version.
It's not hardcoded, subclass can change it via "setDMversion()".
It is hardcoded in the sense that each module is has a hardcoded DM
version. Now, if Verizon changes the DM version to 5 for a phone with
a new firmware, as they did with the VX-8600, the way it is written
now will require a new build of Bitpim that reflects the change. If,
on the other hand, DMv5 is always tried such a change by Verizon will
not break support for the phone in the current build. If DMv5 is
tried on a phone that does not use it at this time it will respond to:
\xFE \x00 \x00 \x00 \x00 \x00 \x00

with:
\x13 \xFE \x00 \x00 \x00 \x00 \x00 \x00

That error can be (and is) caught by a try/except then ignored. The
code can then fall back on DMv4, as it was originally written. An
instance variable could be set indicating DMv5 is not available to
avoid trying it again. This will provide maximum support for
supported phones (and not require additional code in any phone module).
Post by Joe Pham
Post by Nathan Hjelm
operating on a timer should work fine
That was meant for those phone models that may get kicked out of DM
after some amount of time. Obviously, one has to determine, or
guess, how long that is.
I am unsure how long it takes the phone to kick out of DM. The
VX-8600 takes between 10 and 60 seconds (I may have seen longer). One
possible explanation of this variability is that the phone re-enters
DM X seconds after the last command. X is very small (on the order of
seconds) on the VX-9400 and reasonably long on the VX-8600. If you
wish to use a timer I would suggest making it very short (that might
defeat the purpose of using a timer in the first place).
Post by Joe Pham
Post by Nathan Hjelm
trying the command and getting the access denied has no apparent
negative effect.
Not sure what you meant, is that a good or bad thing?
No negative effect means that a command (requiring DM) can be tried
and then, if the phone returns "access denied", be retried after (re)
entering DM.

Also, I do not see the VX-9400 or sendbrewcommand changes.
Joe Pham
2007-07-01 16:38:52 UTC
Permalink
If, on the other hand, DMv5 is always tried such a change by Verizon
will not break support for the phone in the current build.
So you'd rather hardcode it the other way rather than have the
flexibility to do either?
If you wish to use a timer I would suggest making it very short
(that might defeat the purpose of using a timer in the first place).
I don't have access to any of those phones so I can't guess, but 60-
second sounds reasonable.
Also, I do not see the VX-9400 or sendbrewcommand changes.
I have not committed those changes until the LGDMPhone class is
finished. btw, I'm planning to do a test release tomorrow (Monday)
night which can include your patch if you have it ready.

-Joe Pham


_____________________________________________________________
Amazing online psychic readings & more
http://track.netzero.net/s/lc?u=http://tagline.untd.us/fc/Ioyw6ijm46YfjjyYsejhAjq2EQR5IwE2dXYQJ2JezpGIirTkTGXeFO/
Nathan Hjelm
2007-07-01 21:35:47 UTC
Permalink
Post by Joe Pham
If, on the other hand, DMv5 is always tried such a change by Verizon
will not break support for the phone in the current build.
So you'd rather hardcode it the other way rather than have the
flexibility to do either?
Somewhat. I would like bitpim to be able to adjust when the phone does not respond as the programmer expected. Maybe LGDMPhone could catch the permission denied and switch to trying the other DM version. An instance variable could be set to control this behavior. This way it could be had both ways :).

I could write a patch that does this and send it to you to try.
Post by Joe Pham
If you wish to use a timer I would suggest making it very short
(that might defeat the purpose of using a timer in the first place).
I don't have access to any of those phones so I can't guess, but 60-
second sounds reasonable.
Yeah. Should work. I will try it out with a VX-8600 and let you know.
Post by Joe Pham
Also, I do not see the VX-9400 or sendbrewcommand changes.
I have not committed those changes until the LGDMPhone class is
finished. btw, I'm planning to do a test release tomorrow (Monday)
night which can include your patch if you have it ready.
Alright. I will get you a patch this evening.

-Nathan
Joe Pham
2007-07-01 23:09:11 UTC
Permalink
Post by Nathan Hjelm
Maybe LGDMPhone could catch the permission denied and switch to
trying the other DM version.
There's no need for that extra complexity. Just turn on the V5 for
those models that *may* need it.

-Joe Pham



_____________________________________________________________
Click to find local LDS singles for dating and romance
http://track.netzero.net/s/lc?u=http://tagline.untd.us/fc/Ioyw6ijm1mMn4yTsdzjTZvDkFCZr9Z3yo2Z8EteZLj4mBO38a06qZe/
Nathan Hjelm
2007-07-02 03:46:49 UTC
Permalink
Alright, I will send you a patch adding DMv5 to the phones that need it.

-Nathan
Post by Joe Pham
Post by Nathan Hjelm
Maybe LGDMPhone could catch the permission denied and switch to
trying the other DM version.
There's no need for that extra complexity. Just turn on the V5 for
those models that *may* need it.
-Joe Pham
_____________________________________________________________
Click to find local LDS singles for dating and romance
http://track.netzero.net/s/lc?u=http://tagline.untd.us/fc/
Ioyw6ijm1mMn4yTsdzjTZvDkFCZr9Z3yo2Z8EteZLj4mBO38a06qZe/
----------------------------------------------------------------------
---
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
BitPim-devel mailing list
https://lists.sourceforge.net/lists/listinfo/bitpim-devel
Nathan Hjelm
2007-07-02 07:03:02 UTC
Permalink
Some new patches:
lg_dm5_phones_patch:
- Adds VX-9400, T53VZV04, T86VZV03, and T99VZV02 under the modified
LGDMPhone context.

vx9400_update_patch:
- Updated VX-9400 support.

brewsendcommand_update_patch:
- Updated access denied error detection.


Also, I noticed something interesting. It seems the VX-8700 is
capable of writing blocks of size 0x1000 and reading blocks of size
0x400. Using these block sizes greatly improves upload/download
speeds (i.e. 3 kB/sec becomes 14 kB/sec). It might be useful to
define a way to allow modules to manipulate transfer block sizes.
Loading...