Skip to content

feat: add support for CoAP transport#308

Open
Woguagua wants to merge 68 commits into
apache:develop_coapfrom
Woguagua:develop_coap_pub_sub
Open

feat: add support for CoAP transport#308
Woguagua wants to merge 68 commits into
apache:develop_coapfrom
Woguagua:develop_coap_pub_sub

Conversation

@Woguagua

Copy link
Copy Markdown

No description provided.

Woguagua added 30 commits July 23, 2024 11:03
…ssage published by coap can be accepted by mqtt consumer
…riodly, and remove the session which is expired.
…equest, without further processing. But the messages sent by session-write function has not been marked.

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review by github-manager-bot

Summary

Adds CoAP (Constrained Application Protocol) transport support to RocketMQ-MQTT, enabling IoT devices using CoAP to connect to the RocketMQ messaging system. This is a large feature addition (~5600 lines) with new protocol handlers, session management, and test coverage.

Findings

  • [Critical] distribution/conf/meta.conf and distribution/conf/service.conf — These files contain hardcoded internal IP addresses (11.159.23.108, 11.159.23.111, 11.159.23.104) and test credentials (username=test, secretKey=test). These look like development environment values that were accidentally committed. These must be reverted to empty/placeholder values before merge.

  • [Warning] PR Scope: This is a very large PR (~5600 lines, 60+ files). Consider splitting into smaller, reviewable chunks:

    1. CoAP message model and codec (mqtt-common)
    2. CoAP protocol handlers (mqtt-cs)
    3. CoAP upstream processors (mqtt-ds)
    4. Tests
  • [Warning] MqttServer.java — Adding CoAP UDP listener startup needs review for:

    • Port conflict handling
    • Graceful shutdown of the DatagramChannel
    • Resource cleanup on startup failure
  • [Info] Good test coverage — the PR includes unit tests for most new components (CoapDecoder, CoapEncoder, handlers, etc.). This is appreciated for a feature of this size.

  • [Info] CoapMessage.java — The intToByteArray(int) method hardcodes 3-byte arrays. Verify this matches the CoAP spec (RFC 7252) token length constraints.

Suggestions

  1. Must fix: Remove hardcoded IPs and test credentials from config files.
  2. Consider using Eclipse Californium (the reference CoAP implementation for Java) instead of a custom codec, which would reduce maintenance burden and improve spec compliance.
  3. Add a README section documenting the CoAP transport feature, its limitations, and configuration.
  4. Consider adding a feature flag to enable/disable CoAP support, since not all deployments need it.

Automated review by github-manager-bot

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