search

sacOO7 / socketcluster-client-java

Native java and android client for socketcluster framework in node.js
http://socketcluster.io/
Apache License 2.0
94 stars 42 forks source link

Allow user to add custom codecEngine #27

Open AUT0CRAT opened 6 years ago

AUT0CRAT commented 6 years ago

PR for #10 A basic implementation of Binary Codec for JAVA client.

sacOO7 commented 6 years ago

Hi @AUT0CRAT , will take a look at PR and get back to you :+1:

XDex commented 6 years ago

@AUT0CRAT Could you please provide an example of how the codec is used, possibly adding some testing code to Main class? I'm not really sure the codec will work as intended, not with the current Socket implementation supporting only the NV client's WebSocket.sendText() and converting the JSON object to String before sending it.. I think support for binary WebSocket frame sending should be implemented first, prior to adding SC codecs support.. Btw, I'm planning on doing it soon..

sacOO7 commented 6 years ago

Hi @AUT0CRAT , it will be helpful if you can provide working example. I will try to integrate it by trying out send method with binary data.

sacOO7 commented 6 years ago

Hi @XDex , if you know how to use code defined in PR, it will be useful if you can post working example here 👍 or create PR for it.

XDex commented 6 years ago

@sacOO7 Actually, I'm now working on adding support for sc-min-bin codec in my own fork, however I have refactored it to work with Jackson instead of org.json JSON serializer, as implementation is much simpler with jackson-dataformat-msgpack MsgPack plugin. I can raise a PR once I'm done, however I guess many people would prefer to have a more lightweight implementation without binary codecs support, so maybe it makes more sense to publish it as a separate project, WDYT?

sacOO7 commented 6 years ago

Hi @XDex , I don't think making a separate project a good idea. We have already decided to keep one client per language (since too many clients is creating problem for users to choose one) . It's better if we can keep it at one place. It's easy to maintain that way, and also people can raise and find issues at one place. I don't think 3-4 class files for managing codecs will make implementation heavy as a library ..

XDex commented 6 years ago

@sacOO7 Generally I agree with your point, however it's always better to offer users choice when the differences are too big and breaking, which is the case here.. I've noted the differences here, and unfortunately they are quite impacting: ~150 Kb JAR vs ~1.8 Mb, plus breaking API changes..

sacOO7 commented 6 years ago

HI @XDex , do you have any plans to release library with sc codec min? I mean we can conclude on solution which will work for us... Even keeping the library size as small as possible...