Skip to content

MQTT: update event trigger timing#400

Merged
legendecas merged 2 commits intomasterfrom
fix/mqtt-connect-event
Oct 31, 2018
Merged

MQTT: update event trigger timing#400
legendecas merged 2 commits intomasterfrom
fix/mqtt-connect-event

Conversation

@qile222
Copy link
Copy Markdown
Contributor

@qile222 qile222 commented Oct 30, 2018

  • change the trigger timing of the connect event
  • add close event when the connection is disconnected, see mqtt.js

@qile222 qile222 requested a review from yorkie October 30, 2018 07:58
Copy link
Copy Markdown
Contributor

@legendecas legendecas left a comment

Choose a reason for hiding this comment

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

LGTM

@qile222 qile222 requested a review from legendecas October 30, 2018 12:29
client.once('offline', function() {
assert.ok(disconnected);
});
client.once('error', function(err) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If an assertion shall be made here?

@legendecas legendecas merged commit d38adaa into master Oct 31, 2018
@legendecas legendecas deleted the fix/mqtt-connect-event branch October 31, 2018 03:13
yorkie pushed a commit that referenced this pull request Oct 31, 2018
* fix/mqtt-connect-event

* remove abundant error listener
legendecas pushed a commit that referenced this pull request Oct 31, 2018
* build: use CMAKE_INSTALL_PREFIX instead of custom install command (#391)

* build: add dependencies for iotjs.cmake which depends on others (#396)

Within some parallel builds, it occasionally gets a build error on
libmqtt_packet depenedency.

* events: cannot read property 'newListener' of undefined (#397)

* tools: clang-format-7 on travis-ci (#398)

* mqtt: update event trigger timing (#400)

* fix/mqtt-connect-event

* remove abundant error listener

* n-api: code is optional on create error (#399)

* v0.10.13: bump version
@yorkie yorkie mentioned this pull request Nov 6, 2018
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.

2 participants