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.

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

can set_payload function be added to MessageBuilder class #139

Closed
goutham2688 opened this issue Oct 20, 2021 · 3 comments
Closed

can set_payload function be added to MessageBuilder class #139

goutham2688 opened this issue Oct 20, 2021 · 3 comments
Assignees
Labels
fix added A fix was added to an unreleased branch
Milestone

Comments

@goutham2688
Copy link

type of issue: improvement or how to, if the functionality doesn't exist already.

issue: in certain cases I'd be receiving a mqtt message, and in response I'll be updating the status of the request in multiple publishes to the same topic.
I feel instead of initializing the message via MessageBuilder, I'd like to update just the payload.
and use the same msg structure multiple sense.

current working:

// Create a message and publish it
let msg = mqtt::MessageBuilder::new()
    .topic("test")
    .payload("test1")
    .qos(1)
    .finalize();

if let Err(e) = cli.publish(msg) {
    println!("Error sending message: {:?}", e);
}

let msg = mqtt::MessageBuilder::new()
    .topic("test")
    .payload("test2")
    .qos(1)
    .finalize();

if let Err(e) = cli.publish(msg) {
    println!("Error sending message: {:?}", e);
}

proposed look:

let msg = mqtt::MessageBuilder::new()
    .topic("test")
    .payload("test1")
    .qos(1)
    .finalize();

if let Err(e) = cli.publish(msg) {
    println!("Error sending message: {:?}", e);
} 

msg.set_payload("test2")

if let Err(e) = cli.publish(msg) {
    println!("Error sending message: {:?}", e);
}
@fpagliughi
Copy link
Contributor

fpagliughi commented Oct 20, 2021

Well, the cli.publish(msg) call takes ownership of the message, so we can't update the payload after that because the msg is no longer in scope.

The reason to "give" the message to the client for publishing , instead of passing a shared reference, is mostly for the async varieties of the client. When publish() returns, the operation has only been initiated and has not yet been completed. So the msg object couldn't be mutated since the client was still using it. Passing ownership of the message into the client solves this problem.

There is an alternate way to do what you want, though. Use the Topic struct. This is meant for your exact use case: when you want to repeatedly publish messages with different payloads to the same topic. Use it like:

let topic = mqtt::Topic::new(&cli, "test", QOS);
topic.publish("payload1");
topic.publish("payload2");

This does assume that cli is an AsyncClient. If you're using the synchronous Client, I suppose we can make a sync variety, or see if we can make it generic.

@goutham2688
Copy link
Author

Thanks for the explanation :)
I'm currently using the sync client.

@fpagliughi fpagliughi self-assigned this Jan 16, 2022
@fpagliughi fpagliughi added the fix added A fix was added to an unreleased branch label Jan 16, 2022
@fpagliughi fpagliughi added this to the v0.10 milestone Jan 16, 2022
@fpagliughi
Copy link
Contributor

Added SyncTopic for this. Feel free to reopen this issue if there are any further problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix added A fix was added to an unreleased branch
Projects
None yet
Development

No branches or pull requests

2 participants