Skip to content

# feat: Add a new protocol, zr, listening on port 5257. #5309

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

keDaYao
Copy link

@keDaYao keDaYao commented Apr 25, 2024

Trak-iot.protocol.TC04-9.20220817.docx
Support device - "TC-01、TC-02、TC-03、TC-04" | Protocol - zr Protocol | Port - 5257

@tananaev
Copy link
Member

It doesn't look like all of the comments from last PR were addressed.

@tananaev
Copy link
Member

Also maybe it would be easier to split it into several PRs. For example encoder can be separate from the main decoding.

@keDaYao
Copy link
Author

keDaYao commented Apr 26, 2024

Hi, I think this is difficult to split because when the server receives a message from the device, it must reply with different response messages, which requires the encoder to encode the corresponding response messages. Therefore, the encoder code cannot be split.

@tananaev
Copy link
Member

And where exactly are you using encoder for responses? I don't see it.

@keDaYao
Copy link
Author

keDaYao commented Apr 26, 2024

In ZrProtocolDecoder.java, at lines 89, 92, 96,and 99.

@keDaYao
Copy link
Author

keDaYao commented Apr 26, 2024

Could you please re-label the suggestions for code modifications? I'm not sure what needs to be changed, and I would greatly appreciate it.

@tananaev
Copy link
Member

In ZrProtocolDecoder.java, at lines 89, 92, 96,and 99.

It doesn't use encoder. It's the other way around. Did you write this code yourself?

Could you please re-label the suggestions for code modifications? I'm not sure what needs to be changed, and I would greatly appreciate it.

Unfortunately I don't have time. Please check the original PR.

@keDaYao
Copy link
Author

keDaYao commented May 7, 2024

Yes, I wrote the code.When you mentioned encoder, are you referring to ZrProtocolEncoder.java?I just deleted that part of the file.

@tananaev
Copy link
Member

tananaev commented May 7, 2024

Thanks. Let me know once all the other issues are resolved, so we can review it again.

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

Successfully merging this pull request may close these issues.

2 participants