feat: initial implementation up-java 0.1.11 #1 #2
Conversation
create maven project
- dummy implementation - test setup - basic pipelines - documentation
- implementation of send, registerListener and unregisterListener - added test cases for simple use-cases
- added parsing of all MqttUserProperties to uAttributes in receiving messages use-case
- replaced asserts with proper condition and error handling
…tion-implementation feat: initial implementation up-java 0.1.11-SNAPSHOT #1
There was a problem hiding this comment.
Hi @r-vanooyen thank you for the commit. I have a general comment about the way you implemented this transport.
For all other implementations, there is no factory and we implement to a specific communication middleware (ex. vsomeip, zenoh, etc...). I understand that there are many implementations of MQTT5 clients for java and there could be benefits for having a factory HOWEVER I don't think that developers should be exposed with anythign other than specifying which implementation they want the factory to build (HiveMQ, Paho, etc...). That being said the factory should allow you to specify which one you want to build and then return a UTransport object so that we do not need to call anything other than uTransport APIs.
There was a problem hiding this comment.
there were 2 ideas behind it
- The user of this lib can decide whether he wants to use the hiveMq client or maybe other supported mqtt clients (paho or others)
- We as maintainer of the lib do not want to handle all the different mqtt settings a user of the lib might want to choose (ssl, tls, mtls, mqtt-over-websocket, etc.) so we moved the responsibility of setting up the mqtt connection to the user of the lib, and the lib is taking all the responsiblilities and conversions which are defined in uProtocol
I will discuss it Valtech internally, maybe I am the only one with that opinion. If so, I will change it, according to the rust or python implementation
src/main/java/org/eclipse/uprotocol/mqtt/HiveMqMQTT5Client.java
Outdated
Show resolved
Hide resolved
inlined async methods, as the interface methods are now async removed incorrect copy right
…tion-implementation PR feedback
r-vanooyen
changed the title
| LOG.trace("should send a message:\n{}", uMessage); | ||
| CompletableFuture<UStatus> result = new CompletableFuture<>(); | ||
|
|
||
| Mqtt5UserProperties userProperties = Mqtt5UserProperties.builder() |
There was a problem hiding this comment.
user properties should only be added if the uAttribute value is defined. https://github.com/eclipse-uprotocol/up-spec/blob/main/up-l1/mqtt_5.adoc#:~:text=Only%20UAttributes%20that%20are%20used%20in%20the%20message%20are%20included%20in%20the%20MQTT%20header.%20If%20a%20UAttributes%20field%20is%20not%20present%20in%20the%20header%2C%20than%20it%20is%20considered%20not%20used%20when%20recompiling%20the%20UAttributes
There was a problem hiding this comment.
As a check too before creating the user properties, you could validate the uAttributes from the uMessage with this validator from up-java.
There was a problem hiding this comment.
most of the uAttributes are defined as NotNull in up-java
what should be checked for "value is defined" or not?
There was a problem hiding this comment.
If you look at the uAttributes spec you will see that not every field should be set depending on a message type and within those types not every value is mandatory. uAttributes. It is a good question on how to distinguish between the default value or a set value for uAttributes in java. @stevenhartley do you have any suggestions?
There was a problem hiding this comment.
discussed during the call today to address this
src/main/java/org/eclipse/uprotocol/mqtt/HiveMqMQTT5Client.java
Outdated
Show resolved
Hide resolved
sending messages: - added validation for uAttributes - refactor building methods before receiving message: - extract source and sink from MqttUserProperties
…hese to MQTT Headers
initial implementation of up-client-mqtt-java with up-java 0.1.11 and related up-spec