Copilot commented on code in PR #1152:
URL: https://github.com/apache/pekko-connectors/pull/1152#discussion_r2418411067
##########
mqtt-streaming/src/main/scala/org/apache/pekko/stream/connectors/mqtt/streaming/model.scala:
##########
@@ -712,7 +711,7 @@ object MqttCodec {
v.topicFilters.foreach {
case (topicFilter, topicFilterFlags) =>
topicFilter.encode(packetBsb)
- packetBsb.putByte(topicFilterFlags.underlying.toByte)
+ packetBsb.putByte((topicFilterFlags.underlying >> 1).toByte)
Review Comment:
The QoS encoding appears to be shifting in the wrong direction. According to
MQTT v3.1.1 specification section 3.8.3.1, QoS values (0, 1, 2) should be
encoded in bits 1-0 of the byte. Right-shifting by 1 would place QoS values in
bits 2-1 instead. This should likely be a left shift by 1 or no shift at all
depending on how `topicFilterFlags.underlying` represents the QoS value.
```suggestion
packetBsb.putByte(topicFilterFlags.underlying.toByte)
```
##########
mqtt-streaming/src/main/scala/org/apache/pekko/stream/connectors/mqtt/streaming/model.scala:
##########
@@ -978,7 +977,7 @@ object MqttCodec {
: Vector[(Either[DecodeError, String], ControlPacketFlags)] =
if (remainingLen > 0) {
val packetLenAtTopicFilter = v.len
- val topicFilter = (v.decodeString(), ControlPacketFlags(v.getByte
& 0xFF))
+ val topicFilter = (v.decodeString(), ControlPacketFlags((v.getByte
<< 1) & 0xFF))
Review Comment:
The QoS decoding is left-shifting by 1 bit, but this appears inconsistent
with typical MQTT QoS encoding where QoS values occupy the least significant
bits (bits 1-0). If the encoding is right-shifting by 1, then decoding should
also right-shift by 1 to extract the QoS from the correct bit positions. The
shift directions in encode and decode operations should be inverse operations.
```suggestion
val topicFilter = (v.decodeString(),
ControlPacketFlags(v.getByte & 0x03))
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
