cleaner code. about easywsclient HOT 11 CLOSED

I'm definitely open to improvement. I can always review what you come up with, but if you can provide some more information, that would be helpful. Can you propose what the new interface is, and if there are any dependency changes? If the interface changes, it may be better to consider developing it under a "v2" branch while it gets settled out and to create a transition path to help people switch to it.

from easywsclient.

I propose a class only websocket, no namespaces and no websocket code
outside the class.
This means more websockets can be started by the application by various
threads.
I propose a hpp file with the class definition and function definitions.
It is called easywsclient so it should be as easy to use as possible.
I run into some bugs to that i want fixed.
This does not run correctly with me as i don't receive hello3 on the server:

ws->send("hello1");
if (ws->getReadyState() != WebSocket::CLOSED) {
  ws->poll();
  ws->dispatch(handle_message);
}
ws->send("hello2");
if (ws->getReadyState() != WebSocket::CLOSED) {
  ws->poll();
  ws->dispatch(handle_message);
}
ws->send("hello3");

I get back to you soon with my version of the easywsclient.
Weather you want to adopt it as v2 is up to you.

On Mon, Sep 1, 2014 at 3:52 PM, David Baird [email protected]
wrote:

I'm definitely open to improvement. I can always review what you come up
with, but if you can provide some more information, that would be helpful.
Can you propose what the new interface is, and if there are any dependency
changes? If the interface changes, it may be better to consider developing
it under a "v2" branch while it gets settled out and to create a transition
path to help people switch to it.

—
Reply to this email directly or view it on GitHub
#30 (comment).

from easywsclient.

@pbplugge, I did not understand all of your points and have the following questions, and hopefully clarification as well to how the library works:

1. How would eliminating namespaces fix things? If you feel it's a hassle, just type using namespace easywsclient at the top of your code. Is there some other reason that you have for wanting to eliminate namespaces?
2. Threading: It is possible right now for various threads to create websockets. For example, if you want to have 100 threads, each with their own websocket instance, that would be perfectly fine. If you observed otherwise, it is a bug, and please file a ticket for it.
3. You need to call poll() (repeatedly, until the send buffer is emptied). After calling ws->send("hello3"), you must call poll() in order for it to be sent. Actual sending and receiving (over sockets) is handled in the poll() function. It is safe to call ws->poll() without needing to check WebSocket::CLOSED.

I can offer one small word of wisdom: breaking the problem into smaller pieces would be much easier to digest than a full rewrite and a change of interface (but I also don't want to discourage you if you feel there is a good need for such a full rewrite). Based on the observations above, it sounds partly like the documentation just needs some better examples.

There is a particular deficiency that you may be interested in seeing if you can help address, that I've written up here: #31.

Good luck. Looking forward to see what you come up with :)

from easywsclient.

Ok, thanks for your reply.
I did not know about poll.
As a simple user, if i call send, i would expect my message to be send
without having to call another function.
Tonight i will take a look at some other web socket implementations to see
if there is a function call standard that i can take in consideration with
my version.

On Tue, Sep 2, 2014 at 4:55 AM, David Baird [email protected]
wrote:

@pbplugge https://github.com/pbplugge, I did not understand all of your
points and have the following questions, and hopefully clarification as
well to how the library works:

1. How would eliminating namespaces fix things? If you feel it's a hassle,
   just type using namespace easywsclient at the top of your code. Is there
   some other reason that you have for wanting to eliminate namespaces?
2. Threading: It is possible right now for various threads to create
   websockets. For example, if you want to have 100 threads, each with their
   own websocket instance, that would be perfectly fine. If you observed
   otherwise, it is a bug, and please file a ticket for it.
3. You need to call poll() (repeatedly, until the send buffer is
   emptied). After calling ws->send("hello3"), you must call poll() in order
   for it to be sent. Actual sending and receiving (over sockets) is handled
   in the poll() function. It is safe to call ws->poll() without needing to
   check WebSocket::CLOSED.

I can offer one small word of wisdom: breaking the problem into smaller
pieces would be much easier to digest than a full rewrite and a change of
interface (but I also don't want to discourage you if you feel there is a
good need for such a full rewrite). Based on the observations above, it
sounds partly like the documentation just needs some better examples.

There is a particular deficiency that you may be interested in seeing if
you can help address, that I've written up here: #31
#31.

Good luck. Looking forward to see what you come up with :)

—
Reply to this email directly or view it on GitHub
#30 (comment).

from easywsclient.

@pbplugge - not sure if this helps, but there is the W3C spec. I wish I had followed that more closely. That means, instead of a "dispatch()" method, the interface should have a "onmessage" setter. As for "poll()"- I'm open to ideas for improving it. I have thought about it and there are some reasons for why it exists, but I don't entirely like it.

from easywsclient.

I think you might like what i am making then.
Spend another few good hours on it and it takes shape.
Talk to you soon.

On Tue, Sep 2, 2014 at 2:46 PM, David Baird [email protected]
wrote:

@pbplugge https://github.com/pbplugge - not sure if this helps, but
there is the W3C spec http://www.w3.org/TR/websockets/. I wish I had
followed that more closely. That means, instead of a "dispatch()" method,
the interface should have a "onmessage" setter. As for "poll()"- I'm open
to ideas for improving it. I have thought about it and there are some
reasons for why it exists, but I don't entirely like it.

—
Reply to this email directly or view it on GitHub
#30 (comment).

from easywsclient.

Well, i am about done, i hope you will like it.
I have no time now to commit today since i am not any good with git.
I rather test it first coming days with my own software and commit on monday with some help.

I will create a json proxy today and tomorow for my version since i use that too and that is the format used with communication between websockets.

from easywsclient.

FYI. I just committed my code but for the past hours i am trying to make it work with my own application but it won't send the message correctly. That is to say, the server does not receive it. So it needs some more work.

from easywsclient.

My test application works but as soon as i migrate to my project it crashes. That looks to me like an uninitialized variable or memory leak.

My easywsclient disconnects immediately when either of these 2 commands runs:

(i changed char* cast to int but does not make a difference)
setsockopt(sockfd, IPPROTO_TCP, TCP_NODELAY, &flag, sizeof(flag)); // Disable Nagle's algorithm

fcntl(sockfd, F_SETFL, O_NONBLOCK);

If i don't run these commands, the the connection stays alive untill the application quits. But of course i cannot communicate becouse these options are not set.

Hopefully someone has an idear on how to fix this.

from easywsclient.

Now these commands suddenly work.... easywsclient still not... disregard previous post i keep looking.

edit: now it disconnects on.
int ret = send(sockfd, (char*) &txbuf[0], txbuf.size(), 0);
return;

It does not disconnect on:
int ret = send(sockfd, "test", 4, 0);
return;

from easywsclient.

issue seems to be solved.
when i set useMask on true it works.
great, now i can go fast forward.
if you like my version i'll commit.

from easywsclient.