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.

Sign up for GitHub

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

Jump to bottom

Changing Ping interval, added debugging message, fixed Sec-WebSocket-Protocol, and fixed wss #33

Merged
merged 3 commits into from
Mar 1, 2018
Merged

Changing Ping interval, added debugging message, fixed Sec-WebSocket-Protocol, and fixed wss #33

merged 3 commits into from
Mar 1, 2018

Conversation

steven-smpct
Copy link
Contributor

No description provided.

@steven-smpct
Made Websocket PingPong Interval Configurable
0a06327
@steven-smpct
Merge branch 'master' of https://github.com/ChargeTimeEU/Java-OCA-OCPP
6090719
@steven-smpct
Adding debugging message and fixed Sec-WebSocket-Protocol
9f9c9e2 
 - Added debug message for sending and recieving
 - Removed OcppDraft and used example from https://github.com/TooTallNate/Java-WebSocket/wiki/Specifing-a-Sec-WebSocket-Protocol
 - Changed interface for enabling wss, because throws null exception. Setting socket needs to between creating client and connecting
@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 57.062% when pulling 9f9c9e2 on steven-smpct:master into f8df205 on ChargeTimeEU:master.

TVolden
TVolden reviewed Feb 28, 2018
View reviewed changes
Copy link
Member

@TVolden TVolden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for my slow response. It looks good.
I like that you constructor inject the SSLContext, but why did you remove the draft? I'm kinda sad it went from being injected in the constructor, to be newed up in the connect method.

@steven-smpct
Copy link
Contributor Author

Hi Thomas,

The reason I removed the draft was because it wasn't working for me. It may of been because I had a mismatched of the Java-WebSocket library when pulling code and then repaired on the way to the current solution.

I felt the draft increasing code complexity and required you to understand the underlying libraries. This way you keep the abstraction layer and follow examples from the library itself (https://github.com/TooTallNate/Java-WebSocket/wiki/Specifing-a-Sec-WebSocket-Protocol).

Both solutions might work but I didn't have time to test on my system. I prefer my solution because it reduces code complexity and follow standards from the library itself.

Regards,
Steven

@TVolden
Copy link
Member

TVolden commented Mar 1, 2018

Fair enough, I'll take it.

@TVolden TVolden merged commit eb26cf6 into ChargeTimeEU:master Mar 1, 2018
@TVolden
Copy link
Member

TVolden commented Mar 13, 2018

I didn't notice it right away, but all the integration tests are failing on this PR.

@steven-smpct
Copy link
Contributor Author

steven-smpct commented Mar 13, 2018 •
edited
Loading

I don't see the integration tests failing. Below is after mvn clean then mvn test.

[INFO] ------------------------------------------------------------------------
[INFO] Building Java-OCA-OCPP 0.5-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[INFO]
[INFO] --- jacoco-maven-plugin:0.7.9:prepare-agent (prepare-agent) @ all ---
[INFO] argLine set to -javaagent:/home/ubuntu/.m2/repository/org/jacoco/org.jacoco.agent/0.7.9/org.jacoco.agent-0.7.9-runtime.jar=destfile=/home/ubuntu/Work/ocpp-workspace/Java-OCA-OCPP/target/jacoco.exec
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO]
[INFO] Java-OCA-OCPP common ............................... SUCCESS [ 4.374 s]
[INFO] Java-OCA-OCPP v1.6 ................................. SUCCESS [ 2.997 s]
[INFO] Java-OCA-OCPP v1.6 - Integration test .............. SUCCESS [ 1.429 s]
[INFO] Java-OCA-OCPP ...................................... SUCCESS [ 0.210 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 9.128 s
[INFO] Finished at: 2018-03-13T16:56:58-07:00
[INFO] Final Memory: 32M/570M
[INFO] ------------------------------------------------------------------------

@TVolden TVolden mentioned this pull request Apr 5, 2018
Closed
TVolden added a commit that referenced this pull request Apr 7, 2018
@TVolden
Reintroduced changes from @steven-smpct PR #33
357b7ce 
Added same protocol to WebSocketListener.
@TVolden
Copy link
Member

TVolden commented Apr 7, 2018

Sorry for the extreme slow response, but I reintroduced your changes and made them work by adding the protocol to the server as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TVolden TVolden TVolden left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@steven-smpct @coveralls @TVolden