On Mon, Jan 26, 2009 at 06:36:45PM +0100, Nicolas wrote:
> Hi,
>
> I have read the TODO list ; and I have found the screenshot feature.
>
> To win time and see Chris works about OpenSync plugin, I propose this
> patch :)
>
> I begin to be familiar with barry software.
Thanks very much!
> I think that the code is quiet clean ; but you should fix :
> - swap byte
> - throw exceptions
>
> I have tested the code with BlackBerry Storm 9530 (with accelometer
> feature).
> The patch should detect the screenshot size and orientation !
I have a few quibbles with this patch, and if you have time to fix them,
that would be great. If not, just let me know, and I can do it.
1) any code that accesses protocol values should be careful about endian
issues... I see that you have done this in some places, which is great.
The new function:
+unsigned int JLPacket::Size()
+{
+ Protocol::CheckSize(m_receive, SB_JLRESPONSE_HEADER_SIZE);
+ MAKE_JLPACKET(rpack, m_receive);
+ return rpack->u.response.expect;
+}
should return btohs(rpack->u.response.expect);
Note that "btohs" stands for "Blackberry to Host, short", and is supposed
to be analogous to the standard network functions ntohs() and friends.
2) JavaLoader::GetScreenshot() is still using hard coded values, such as
0x64, which has a define for it (ACK is probably the right define)
3) Note that tabs are 8 characters, so reindenting code in JLPacket::PacketData
will just have to be undone again, by one of us. :-)
4) I think we should stick to the command line syntax available in the Windows
javaloader, similar to Josh Kropf's patches to bjavaloader, to make it
easy for users to switch between Linux and Windows and use the same
commands. So instead of a new command 'bscreenshot', we should add a
'screenshot' command argument to bjavaloader.
The windows syntax is:
javaloader screenshot <.bmp file>
5) I'd prefer to have a struct for the BMP header in bscreenshot. Endian
issues should be handled there too, to keep Barry completely
cross platform. The struct is optional, but the endian issues
should be addressed.
My goal is to have the code somewhat self documenting,
and I believe structs help with that goal more than hex data.
While this doesn't matter too much for the BMP format, my goal is
that someone should be able to read through src/protostructs.h and
write english documentation about the Blackberry USB protocol, without
too much trouble. They would have to cross reference the code
to find out *when* the packets are sent, but they wouldn't have
to figure out *what* is being sent. :-)
Otherwise, the patch looks good.
Thanks!
- Chris
------------------------------------------------------------------------------
This SF.net email is sponsored by:
SourcForge Community
SourceForge wants to tell your story.
http://p.sf.net/sfu/sf-spreadtheword
_______________________________________________
Barry-devel mailing list
Barry-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/barry-devel
