{"action":"created","comment":{"url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/pulls/comments/75919813","id":75919813,"diff_hunk":"@@ -514,9 +549,20 @@ function ThingShadowsClient(deviceOptions, thingShadowOptions) {\n          // which the application will need to filter out.\n          //\n          if (thingShadows[thingName].persistentSubscribe === true) {\n-            this._handleSubscriptions(thingName, ['update', 'get', 'delete'], ['accepted', 'rejected'],\n-               'subscribe');\n+            topicsObject.push({\n+               operations: ['update', 'get', 'delete'],\n+               statii: ['accepted', 'rejected'],\n+            });\n+         }\n+\n+         if (topicsObject.length > 0) {\n+            this._handleSubscriptions(thingName, topicsObject, 'subscribe', callback);","path":"thing/index.js","position":168,"original_position":168,"commit_id":"8c183c7d0647fe81b19619f18b6b09d884eb52f5","original_commit_id":"8c183c7d0647fe81b19619f18b6b09d884eb52f5","user":{"login":"rongsaws","id":19155175,"avatar_url":"https://avatars.githubusercontent.com/u/19155175?v=3","gravatar_id":"","url":"https://api.github.com/users/rongsaws","html_url":"https://github.com/rongsaws","followers_url":"https://api.github.com/users/rongsaws/followers","following_url":"https://api.github.com/users/rongsaws/following{/other_user}","gists_url":"https://api.github.com/users/rongsaws/gists{/gist_id}","starred_url":"https://api.github.com/users/rongsaws/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rongsaws/subscriptions","organizations_url":"https://api.github.com/users/rongsaws/orgs","repos_url":"https://api.github.com/users/rongsaws/repos","events_url":"https://api.github.com/users/rongsaws/events{/privacy}","received_events_url":"https://api.github.com/users/rongsaws/received_events","type":"User","site_admin":false},"body":"From encapsulation point of view, pending should probably better be manipulated here, also you probably don't want to return granted etc in this register api, as whoever calls register() most likely won't care about what topics are accepted or not, what they do care is whether or not register() is successful. \r\n\r\nI would recommend to do something like this here\r\n\r\n```\r\nthis._handleSubscriptions(thingName, topicsObject, 'subscribe', function(granted) {\r\n        var res = \"Not all subscriptions were granted\";\r\n        if (all topics are granted) {\r\n               res = null;\r\n               this.pending = false;  \r\n        }\r\n        if (!undefined(callback)) {\r\n             callback(res);\r\n        }\r\n});\r\n```\r\n\r\n\r\nAlso note _handleSubscriptions() does subscribe and unsubscribe, but for unsubscribe, there's no granted argument to the callback function.","created_at":"2016-08-23T18:04:42Z","updated_at":"2016-08-23T18:04:42Z","html_url":"https://github.com/aws/aws-iot-device-sdk-js/pull/83#discussion_r75919813","pull_request_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/pulls/83","_links":{"self":{"href":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/pulls/comments/75919813"},"html":{"href":"https://github.com/aws/aws-iot-device-sdk-js/pull/83#discussion_r75919813"},"pull_request":{"href":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/pulls/83"}}},"pull_request":{"url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/pulls/83","id":81821662,"html_url":"https://github.com/aws/aws-iot-device-sdk-js/pull/83","diff_url":"https://github.com/aws/aws-iot-device-sdk-js/pull/83.diff","patch_url":"https://github.com/aws/aws-iot-device-sdk-js/pull/83.patch","issue_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/issues/83","number":83,"state":"open","locked":false,"title":"Adding callback to thingShadow.register","user":{"login":"Torsph","id":1153104,"avatar_url":"https://avatars.githubusercontent.com/u/1153104?v=3","gravatar_id":"","url":"https://api.github.com/users/Torsph","html_url":"https://github.com/Torsph","followers_url":"https://api.github.com/users/Torsph/followers","following_url":"https://api.github.com/users/Torsph/following{/other_user}","gists_url":"https://api.github.com/users/Torsph/gists{/gist_id}","starred_url":"https://api.github.com/users/Torsph/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Torsph/subscriptions","organizations_url":"https://api.github.com/users/Torsph/orgs","repos_url":"https://api.github.com/users/Torsph/repos","events_url":"https://api.github.com/users/Torsph/events{/privacy}","received_events_url":"https://api.github.com/users/Torsph/received_events","type":"User","site_admin":false},"body":"- Update the thing register function with a completion callback\r\n- Add new test for the different states (ignoreDelta and persistentSubscribe)\r\n- Updated README.md to reflect the new changes\r\n\r\nFixes issue #80 if I understood the aws and mqtt implementation correct. The new code waits for the mqtt lib to invoke the callback on _ignoreDeltas_ and _persistentSubscribe_ topics. When the mqtt lib has invoked both callbacks, will the register function invoke the register callback.\r\n\r\nMaybe need for some more test cases, but please review and provide with feedback :) ","created_at":"2016-08-18T14:05:47Z","updated_at":"2016-08-23T18:04:42Z","closed_at":null,"merged_at":null,"merge_commit_sha":"1ba6b74dc8cc727ba5b13d6b5a52e9fe58a9f052","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/pulls/83/commits","review_comments_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/pulls/83/comments","review_comment_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/pulls/comments{/number}","comments_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/issues/83/comments","statuses_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/statuses/8c183c7d0647fe81b19619f18b6b09d884eb52f5","head":{"label":"Torsph:add_callback_to_thingshadow_register","ref":"add_callback_to_thingshadow_register","sha":"8c183c7d0647fe81b19619f18b6b09d884eb52f5","user":{"login":"Torsph","id":1153104,"avatar_url":"https://avatars.githubusercontent.com/u/1153104?v=3","gravatar_id":"","url":"https://api.github.com/users/Torsph","html_url":"https://github.com/Torsph","followers_url":"https://api.github.com/users/Torsph/followers","following_url":"https://api.github.com/users/Torsph/following{/other_user}","gists_url":"https://api.github.com/users/Torsph/gists{/gist_id}","starred_url":"https://api.github.com/users/Torsph/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Torsph/subscriptions","organizations_url":"https://api.github.com/users/Torsph/orgs","repos_url":"https://api.github.com/users/Torsph/repos","events_url":"https://api.github.com/users/Torsph/events{/privacy}","received_events_url":"https://api.github.com/users/Torsph/received_events","type":"User","site_admin":false},"repo":{"id":65970956,"name":"aws-iot-device-sdk-js","full_name":"Torsph/aws-iot-device-sdk-js","owner":{"login":"Torsph","id":1153104,"avatar_url":"https://avatars.githubusercontent.com/u/1153104?v=3","gravatar_id":"","url":"https://api.github.com/users/Torsph","html_url":"https://github.com/Torsph","followers_url":"https://api.github.com/users/Torsph/followers","following_url":"https://api.github.com/users/Torsph/following{/other_user}","gists_url":"https://api.github.com/users/Torsph/gists{/gist_id}","starred_url":"https://api.github.com/users/Torsph/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Torsph/subscriptions","organizations_url":"https://api.github.com/users/Torsph/orgs","repos_url":"https://api.github.com/users/Torsph/repos","events_url":"https://api.github.com/users/Torsph/events{/privacy}","received_events_url":"https://api.github.com/users/Torsph/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/Torsph/aws-iot-device-sdk-js","description":"SDK for connecting to AWS IoT from a device using JavaScript/Node.js","fork":true,"url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js","forks_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/forks","keys_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/teams","hooks_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/hooks","issue_events_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/issues/events{/number}","events_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/events","assignees_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/assignees{/user}","branches_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/branches{/branch}","tags_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/tags","blobs_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/git/refs{/sha}","trees_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/statuses/{sha}","languages_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/languages","stargazers_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/stargazers","contributors_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/contributors","subscribers_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/subscribers","subscription_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/subscription","commits_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/commits{/sha}","git_commits_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/git/commits{/sha}","comments_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/comments{/number}","issue_comment_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/issues/comments{/number}","contents_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/contents/{+path}","compare_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/merges","archive_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/downloads","issues_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/issues{/number}","pulls_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/pulls{/number}","milestones_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/milestones{/number}","notifications_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/labels{/name}","releases_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/releases{/id}","deployments_url":"https://api.github.com/repos/Torsph/aws-iot-device-sdk-js/deployments","created_at":"2016-08-18T06:24:36Z","updated_at":"2016-08-18T06:24:37Z","pushed_at":"2016-08-22T15:46:08Z","git_url":"git://github.com/Torsph/aws-iot-device-sdk-js.git","ssh_url":"git@github.com:Torsph/aws-iot-device-sdk-js.git","clone_url":"https://github.com/Torsph/aws-iot-device-sdk-js.git","svn_url":"https://github.com/Torsph/aws-iot-device-sdk-js","homepage":"","size":244,"stargazers_count":0,"watchers_count":0,"language":"JavaScript","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"aws:master","ref":"master","sha":"aaec88d362df2c5c274dc130aaf299eda654e319","user":{"login":"aws","id":2232217,"avatar_url":"https://avatars.githubusercontent.com/u/2232217?v=3","gravatar_id":"","url":"https://api.github.com/users/aws","html_url":"https://github.com/aws","followers_url":"https://api.github.com/users/aws/followers","following_url":"https://api.github.com/users/aws/following{/other_user}","gists_url":"https://api.github.com/users/aws/gists{/gist_id}","starred_url":"https://api.github.com/users/aws/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aws/subscriptions","organizations_url":"https://api.github.com/users/aws/orgs","repos_url":"https://api.github.com/users/aws/repos","events_url":"https://api.github.com/users/aws/events{/privacy}","received_events_url":"https://api.github.com/users/aws/received_events","type":"Organization","site_admin":false},"repo":{"id":43452375,"name":"aws-iot-device-sdk-js","full_name":"aws/aws-iot-device-sdk-js","owner":{"login":"aws","id":2232217,"avatar_url":"https://avatars.githubusercontent.com/u/2232217?v=3","gravatar_id":"","url":"https://api.github.com/users/aws","html_url":"https://github.com/aws","followers_url":"https://api.github.com/users/aws/followers","following_url":"https://api.github.com/users/aws/following{/other_user}","gists_url":"https://api.github.com/users/aws/gists{/gist_id}","starred_url":"https://api.github.com/users/aws/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aws/subscriptions","organizations_url":"https://api.github.com/users/aws/orgs","repos_url":"https://api.github.com/users/aws/repos","events_url":"https://api.github.com/users/aws/events{/privacy}","received_events_url":"https://api.github.com/users/aws/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/aws/aws-iot-device-sdk-js","description":"SDK for connecting to AWS IoT from a device using JavaScript/Node.js","fork":false,"url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js","forks_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/forks","keys_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/keys{/key_id}","collaborators_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/teams","hooks_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/hooks","issue_events_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/issues/events{/number}","events_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/events","assignees_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/assignees{/user}","branches_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/branches{/branch}","tags_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/tags","blobs_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/git/refs{/sha}","trees_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/git/trees{/sha}","statuses_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/statuses/{sha}","languages_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/languages","stargazers_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/stargazers","contributors_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/contributors","subscribers_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/subscribers","subscription_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/subscription","commits_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/commits{/sha}","git_commits_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/git/commits{/sha}","comments_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/comments{/number}","issue_comment_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/issues/comments{/number}","contents_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/contents/{+path}","compare_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/compare/{base}...{head}","merges_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/merges","archive_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/downloads","issues_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/issues{/number}","pulls_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/pulls{/number}","milestones_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/milestones{/number}","notifications_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/labels{/name}","releases_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/releases{/id}","deployments_url":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/deployments","created_at":"2015-09-30T18:52:35Z","updated_at":"2016-08-22T19:44:31Z","pushed_at":"2016-08-22T15:46:08Z","git_url":"git://github.com/aws/aws-iot-device-sdk-js.git","ssh_url":"git@github.com:aws/aws-iot-device-sdk-js.git","clone_url":"https://github.com/aws/aws-iot-device-sdk-js.git","svn_url":"https://github.com/aws/aws-iot-device-sdk-js","homepage":"","size":249,"stargazers_count":280,"watchers_count":280,"language":"JavaScript","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":90,"mirror_url":null,"open_issues_count":19,"forks":90,"open_issues":19,"watchers":280,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/pulls/83"},"html":{"href":"https://github.com/aws/aws-iot-device-sdk-js/pull/83"},"issue":{"href":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/issues/83"},"comments":{"href":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/issues/83/comments"},"review_comments":{"href":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/pulls/83/comments"},"review_comment":{"href":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/pulls/83/commits"},"statuses":{"href":"https://api.github.com/repos/aws/aws-iot-device-sdk-js/statuses/8c183c7d0647fe81b19619f18b6b09d884eb52f5"}}}}
{"actor":{"display_login":"rongsaws"}}