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

Thanks for code. Maybe you interested in proposed improvements: #10

Open
igor-- opened this issue Apr 10, 2013 · 8 comments
Open

Thanks for code. Maybe you interested in proposed improvements: #10

igor-- opened this issue Apr 10, 2013 · 8 comments

Comments

@igor--
Copy link

igor-- commented Apr 10, 2013

  • no need HappyDataInputStream extends DataInputStream,
    if you use DataInputStream readFully method: stream.readFully(...) to read from input stream exactly some number
    of bytes (not less as it can be if you use read() )

  • you use double arithmetic in the code (seems it's java script way But in Java we can work with bits fields.
    So instead:
    frame[1] = (byte) (masked | 127);
    frame[2] = (byte) (((int) Math.floor(length / Math.pow(2, 56))) & BYTE);
    frame[3] = (byte) (((int) Math.floor(length / Math.pow(2, 48))) & BYTE);
    frame[4] = (byte) (((int) Math.floor(length / Math.pow(2, 40))) & BYTE);
    frame[5] = (byte) (((int) Math.floor(length / Math.pow(2, 32))) & BYTE);
    frame[6] = (byte) (((int) Math.floor(length / Math.pow(2, 24))) & BYTE);
    frame[7] = (byte) (((int) Math.floor(length / Math.pow(2, 16))) & BYTE);
    frame[8] = (byte) (((int) Math.floor(length / Math.pow(2, 8))) & BYTE);
    frame[9] = (byte) (length & BYTE);

    can be used:
    frame[1] = (byte) (masked | 127);
    //frame[2] = (byte) (0);
    //frame[3] = (byte) (0);
    //frame[4] = (byte) (0);
    //frame[5] = (byte) (0);
    frame[6] = (byte) ((length >> 24) & 0xFF);
    frame[7] = (byte) ((length >> 16) & 0xFF);
    frame[8] = (byte) ((length >> 8) & 0xFF);
    frame[9] = (byte) (length & 0xFF);

    • variable 'length' has int type, so contains only 4 bytes, so
      frame[2]..frame[5] will be always 0.
      (your code ported from java script and wrote as length can contains 8 bytes)

    • you can use DataInputStream standard function to read 2 and 8 bytes integer, likes:
      length = stream.readUnsignedShort(); // read 2 bytes length

      long length8 = stream.readLong(); // read 8 bytes length
      if( length8 > Integer.MAX_VALUE )
      throw new IOException("too big frame length");
      length = (int)length8;

instead of your method: byteArrayToLong()

@codebutler
Copy link
Owner

Thanks igor, would you be able to submit a pull request?

@igor--
Copy link
Author

igor-- commented May 28, 2013

Hi !

For me it was learning task.
Really I make clone:

  • removed Android specific thread (replaces to ExecutorService), so now it
    works in standard java (tested in Windows)
  • in bases of your code added server socket implementation.
    So now it's both client and server (see ClientTest.java, ServerTest.java)
  • added naive http support (enough to web handshake)

You free use the code as you want (MIT licence)

Please see attachment

Regards,
Igor

On Tue, May 28, 2013 at 12:49 AM, Eric Butler notifications@git.luolix.topwrote:

Thanks igor, would you be able to submit a pull request?


Reply to this email directly or view it on GitHubhttps://github.com//issues/10#issuecomment-18515911
.

@codebutler
Copy link
Owner

Hi @igor--, your attachment appears to be missing?
thanks.

@igor--
Copy link
Author

igor-- commented May 28, 2013

Opps,

In attachment was sources in zip file !

Seems the mail have some filters
How I can send it ?

Or maybe really submit it as clone in github ?

Igor

On Tue, May 28, 2013 at 8:09 PM, Eric Butler notifications@git.luolix.topwrote:

Hi @igor-- https://github.com/igor--, your attachment appears to be
missing?
thanks.


Reply to this email directly or view it on GitHubhttps://github.com//issues/10#issuecomment-18565053
.

@codebutler
Copy link
Owner

a pull request is definitely easiest for me, cheers.
https://help.github.com/articles/creating-a-pull-request

On Tue, May 28, 2013 at 10:57 AM, igor-- notifications@github.com wrote:

Opps,

In attachment was sources in zip file !

Seems the mail have some filters
How I can send it ?

Or maybe really submit it as clone in github ?

Igor

On Tue, May 28, 2013 at 8:09 PM, Eric Butler notifications@git.luolix.topwrote:

Hi @igor-- https://github.com/igor--, your attachment appears to be
missing?
thanks.


Reply to this email directly or view it on GitHub<
https://github.com/codebutler/android-websockets/issues/10#issuecomment-18565053>

.


Reply to this email directly or view it on GitHubhttps://github.com//issues/10#issuecomment-18568306
.

@igor--
Copy link
Author

igor-- commented May 28, 2013

Hi Eric !

I create new project and commit the sources, please see:

https://github.com/igor--/tinywebsocket

Regards,
Igor

On Tue, May 28, 2013 at 9:01 PM, Eric Butler notifications@git.luolix.topwrote:

a pull request is definitely easiest for me, cheers.
https://help.github.com/articles/creating-a-pull-request

On Tue, May 28, 2013 at 10:57 AM, igor-- notifications@github.com
wrote:

Opps,

In attachment was sources in zip file !

Seems the mail have some filters
How I can send it ?

Or maybe really submit it as clone in github ?

Igor

On Tue, May 28, 2013 at 8:09 PM, Eric Butler notifications@git.luolix.topwrote:

Hi @igor-- https://github.com/igor--, your attachment appears to be
missing?
thanks.


Reply to this email directly or view it on GitHub<

https://github.com/codebutler/android-websockets/issues/10#issuecomment-18565053>

.


Reply to this email directly or view it on GitHub<
https://github.com/codebutler/android-websockets/issues/10#issuecomment-18568306>

.


Reply to this email directly or view it on GitHubhttps://github.com//issues/10#issuecomment-18568564
.

@codebutler
Copy link
Owner

Thanks! I'll look into integrating your work. I had considered removing the Android-specific code, I like the name "tinywebsockets".

@igor--
Copy link
Author

igor-- commented May 28, 2013

OK
Thanks.

On Tue, May 28, 2013 at 9:54 PM, Eric Butler notifications@git.luolix.topwrote:

Thanks! I'll look into integrating your work. I had considered removing
the Android-specific code, I like the name "tinywebsockets".


Reply to this email directly or view it on GitHubhttps://github.com//issues/10#issuecomment-18571924
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants