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

MQTT-support #766

Merged
merged 23 commits into from
Feb 12, 2025
Merged

MQTT-support #766

merged 23 commits into from
Feb 12, 2025

Conversation

kickster97
Copy link
Member

@kickster97 kickster97 commented Aug 26, 2024 •
edited
Loading

WHAT is this pull request doing?

An MQTT-session inherits AMQP::Queue and extends with MQTT functionality.
MQTT exchange(topic) will only accept activity from an MQTT client, that will be responsible for routing messages to the MQTT-session.
The mqtt exchange handles MQTT routing keys etc. without needing to do conversions.

NOTE
The first version of MQTT Support does not support binding an AMQP exchange to and MQTT exchange. (Undefined behaviour if done)
In a separate PR we need to extract the AMQP exchange types into the AMQP namespace and in turn be able to differentiate between an AMQP::Destination and an MQTT::Destiantion when calling bind.
With that in place we will be able to safely do exchange to exchange bindings.

Docs for the website

HOW can this pull request be tested?

Specs have been migrated from Myra and fully cover the MQTT protocol, run with crystal spec.
Connect with mqtt client library of your choice and test your usage against this branch

spuun
spuun reviewed Sep 5, 2024
View reviewed changes
spuun
spuun reviewed Sep 5, 2024
View reviewed changes
spuun
spuun reviewed Sep 16, 2024
View reviewed changes
spuun
spuun reviewed Sep 16, 2024
View reviewed changes
spuun
spuun reviewed Sep 16, 2024
View reviewed changes
spuun
spuun reviewed Sep 16, 2024
View reviewed changes
spuun
spuun reviewed Sep 16, 2024
View reviewed changes
spuun
spuun reviewed Sep 24, 2024
View reviewed changes
@spuun
Copy link
Member

spuun commented Sep 24, 2024

I think you can (and maybe even should) build clean session by reusing the auto delete feature of queues.

@kickster97 kickster97 force-pushed the mqtt-poc branch 2 times, most recently from 787840d to 4e747f2 Compare September 27, 2024 07:19
@spuun
Copy link
Member

spuun commented Oct 12, 2024

About #803 (comment)

That connect spec passes, but for the wrong reason. Or well, the socket is closed but not because it receives the wrong packet type, but because of the bug that #803 will fix. When that bug is fixed i still think that the spec will pass, but for the right reason.

I guess the spec could be changed to send a PUBLISH instead (the packet must be greater than 8 byte to not trigger the bug).

@spuun
Copy link
Member

spuun commented Oct 12, 2024

After giving it a second thought I've concluded that we should change the spec to use some other packet type.

@kickster97
Copy link
Member Author

After giving it a second thought I've concluded that we should change the spec to use some other packet type.

ok I will change the spec then! thanks :)

@carlhoerberg
Copy link
Member

Often it's more efficient to yield than to capture and then call a block.

spuun
spuun reviewed Oct 21, 2024
View reviewed changes
spuun
spuun reviewed Oct 21, 2024
View reviewed changes
spuun
spuun reviewed Oct 22, 2024
View reviewed changes
spuun
spuun reviewed Oct 23, 2024
View reviewed changes
spuun
spuun reviewed Oct 23, 2024
View reviewed changes
spuun
spuun reviewed Oct 23, 2024
View reviewed changes
spuun
spuun reviewed Oct 30, 2024
View reviewed changes
kickster97 and others added 19 commits February 11, 2025 14:33
@kickster97
rebase
7f3a41e
@kickster97
add comment about keepalive time
25df83f
@kickster97
not have two vhoststore classes
e92c835
@kickster97
general fixes
9b96478
@kickster97
ameba
9ce1fc3
@kickster97
adapt old spec
2145a32
@kickster97 @spuun
Update spec/mqtt/integrations/message_qos_spec.cr
95d5514 
Co-authored-by: Jon Börjesson <[email protected]>
@kickster97
remove old obsolete spec
18161d3
@kickster97 @spuun
Update src/lavinmq/name_validator.cr
a22cd30 
Co-authored-by: Jon Börjesson <[email protected]>
@kickster97
namevalidator fix
162e040
@kickster97
fix default vhost
37eb9b2
@kickster97 @spuun
Update src/lavinmq/mqtt/connection_factory.cr
7a0e6c3 
Co-authored-by: Jon Börjesson <[email protected]>
@kickster97 @spuun
Update src/lavinmq/mqtt/connection_factory.cr
bf7941b 
Co-authored-by: Jon Börjesson <[email protected]>
@spuun @kickster97
Remove unecessary file opening
5d2fd7b 
Also removes bad String convertion of body
@kickster97
remove redundant test and add test and adapt code for mqtt-3.3.1-9
230b45a
@kickster97
fixup retained message spec
102288b
@kickster97
update [MQTT-3.3.1-9] spec
64a00eb
@kickster97
fix review
3f0da4f
@kickster97
rebase authenticator
b968f5d
@spuun spuun self-requested a review February 12, 2025 09:00
@kickster97 kickster97 requested a review from spuun February 12, 2025 09:00
spuun
spuun approved these changes Feb 12, 2025
View reviewed changes
Copy link
Member

@spuun spuun left a comment

Choose a reason for hiding this comment

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

It's time to merge

@kickster97 kickster97 dismissed carlhoerberg’s stale review February 12, 2025 09:09

Some requested changes will be made in separate PRs

@kickster97 kickster97 merged commit 22445e0 into main Feb 12, 2025
21 of 25 checks passed
@kickster97 kickster97 deleted the mqtt-poc branch February 12, 2025 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@snichme snichme snichme approved these changes

@spuun spuun spuun approved these changes

@carlhoerberg carlhoerberg Awaiting requested review from carlhoerberg

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.2
Development

Successfully merging this pull request may close these issues.
4 participants
@kickster97 @spuun @carlhoerberg @snichme