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

async minor #9

Merged
merged 2 commits into from
Mar 15, 2020
Merged

async minor #9

merged 2 commits into from
Mar 15, 2020

Conversation

ringabout
Copy link
Collaborator

No description provided.

@ringabout
Copy link
Collaborator Author

Is this expected behaviour?
image

@ThomasTJdev
Copy link
Collaborator

Is this expected behaviour?

Please elaborate, what you mean? Yes, it looks as correct mqtt packages received listening on global topic #.

@ringabout
Copy link
Collaborator Author

I just run test in when isMainModule

when isMainModule:
  proc flop() {.async.} =
    let ctx = newMqttCtx("hallo")

    #ctx.set_host("test.mosquitto.org", 1883)

    ctx.set_host("test.mosquitto.org", 8883, true)

    await ctx.start()
    proc on_data(topic: string, message: string) =
      echo "got ", topic, ": ", message

    await ctx.subscribe("#", 2, on_data)
    await ctx.publish("test1", "hallo", 2)
    await sleepAsync 1000
    await ctx.close()

  waitFor flop()

@ThomasTJdev
Copy link
Collaborator

ThomasTJdev commented Mar 15, 2020

The listening on # seems fine, but your publish on test1 has not gone through. I have actually never testet against test.mosquitto.org - only on my own servers.

await ctx.publish("test1", "hallo", 2, true) # Insert true condition to wait for package confirmation (4-way handshake).
await sleepAsync 5000 # Or increase time.

@ThomasTJdev ThomasTJdev merged commit ca6e4c9 into master Mar 15, 2020
@@ -449,8 +449,8 @@ proc runConnect(ctx: MqttCtx) {.async.} =
ctx.state = Error
let ok = await ctx.sendConnect()
if ok:
asyncCheck ctx.runRx()
asyncCheck ctx.runPing()
await ctx.runRx()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I made a mistake. We need the asyncCheck.

@ringabout
Copy link
Collaborator Author

@zevv sorry, I made a mistake. We should revert "asyncCheck ctx.sendDisconnect()" to "discard await ctx.sendDisconnect()"
As for other changes,the use of {.async.} is a heap allocation,so sometimes {.async.} is expensive.Maybe we can avoid it.
https://github.com/dom96/jester/pull/236/files

ThomasTJdev added a commit that referenced this pull request Mar 16, 2020
@zevv
Copy link
Owner

zevv commented Mar 16, 2020 via email

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.

None yet

3 participants