{"action":"created","issue":{"url":"https://api.github.com/repos/athackst/robotiq/issues/1","repository_url":"https://api.github.com/repos/athackst/robotiq","labels_url":"https://api.github.com/repos/athackst/robotiq/issues/1/labels{/name}","comments_url":"https://api.github.com/repos/athackst/robotiq/issues/1/comments","events_url":"https://api.github.com/repos/athackst/robotiq/issues/1/events","html_url":"https://github.com/athackst/robotiq/pull/1","id":192115847,"number":1,"title":"Feature/robotiq review","user":{"login":"athackst","id":6098197,"avatar_url":"https://avatars.githubusercontent.com/u/6098197?v=3","gravatar_id":"","url":"https://api.github.com/users/athackst","html_url":"https://github.com/athackst","followers_url":"https://api.github.com/users/athackst/followers","following_url":"https://api.github.com/users/athackst/following{/other_user}","gists_url":"https://api.github.com/users/athackst/gists{/gist_id}","starred_url":"https://api.github.com/users/athackst/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/athackst/subscriptions","organizations_url":"https://api.github.com/users/athackst/orgs","repos_url":"https://api.github.com/users/athackst/repos","events_url":"https://api.github.com/users/athackst/events{/privacy}","received_events_url":"https://api.github.com/users/athackst/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"hongkai-dai","id":6314000,"avatar_url":"https://avatars.githubusercontent.com/u/6314000?v=3","gravatar_id":"","url":"https://api.github.com/users/hongkai-dai","html_url":"https://github.com/hongkai-dai","followers_url":"https://api.github.com/users/hongkai-dai/followers","following_url":"https://api.github.com/users/hongkai-dai/following{/other_user}","gists_url":"https://api.github.com/users/hongkai-dai/gists{/gist_id}","starred_url":"https://api.github.com/users/hongkai-dai/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hongkai-dai/subscriptions","organizations_url":"https://api.github.com/users/hongkai-dai/orgs","repos_url":"https://api.github.com/users/hongkai-dai/repos","events_url":"https://api.github.com/users/hongkai-dai/events{/privacy}","received_events_url":"https://api.github.com/users/hongkai-dai/received_events","type":"User","site_admin":false},"assignees":[{"login":"hongkai-dai","id":6314000,"avatar_url":"https://avatars.githubusercontent.com/u/6314000?v=3","gravatar_id":"","url":"https://api.github.com/users/hongkai-dai","html_url":"https://github.com/hongkai-dai","followers_url":"https://api.github.com/users/hongkai-dai/followers","following_url":"https://api.github.com/users/hongkai-dai/following{/other_user}","gists_url":"https://api.github.com/users/hongkai-dai/gists{/gist_id}","starred_url":"https://api.github.com/users/hongkai-dai/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hongkai-dai/subscriptions","organizations_url":"https://api.github.com/users/hongkai-dai/orgs","repos_url":"https://api.github.com/users/hongkai-dai/repos","events_url":"https://api.github.com/users/hongkai-dai/events{/privacy}","received_events_url":"https://api.github.com/users/hongkai-dai/received_events","type":"User","site_admin":false}],"milestone":null,"comments":1,"created_at":"2016-11-28T21:00:43Z","updated_at":"2016-11-28T22:14:04Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/athackst/robotiq/pulls/1","html_url":"https://github.com/athackst/robotiq/pull/1","diff_url":"https://github.com/athackst/robotiq/pull/1.diff","patch_url":"https://github.com/athackst/robotiq/pull/1.patch"},"body":"Hi @hongkai-dai @arshan\r\n\r\nThis is an old open PR that I have out there.  It should mostly follow ROS style guides and not Google.  Please look over and add feedback.  The robotiq repo was waiting on the license file attachment from me (included in this), but otherwise approved it already.  \r\n\r\n--Allison\n\n<!-- Reviewable:start -->\n---\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/athackst/robotiq/1)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/athackst/robotiq/issues/comments/263411516","html_url":"https://github.com/athackst/robotiq/pull/1#issuecomment-263411516","issue_url":"https://api.github.com/repos/athackst/robotiq/issues/1","id":263411516,"user":{"login":"hongkai-dai","id":6314000,"avatar_url":"https://avatars.githubusercontent.com/u/6314000?v=3","gravatar_id":"","url":"https://api.github.com/users/hongkai-dai","html_url":"https://github.com/hongkai-dai","followers_url":"https://api.github.com/users/hongkai-dai/followers","following_url":"https://api.github.com/users/hongkai-dai/following{/other_user}","gists_url":"https://api.github.com/users/hongkai-dai/gists{/gist_id}","starred_url":"https://api.github.com/users/hongkai-dai/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hongkai-dai/subscriptions","organizations_url":"https://api.github.com/users/hongkai-dai/orgs","repos_url":"https://api.github.com/users/hongkai-dai/repos","events_url":"https://api.github.com/users/hongkai-dai/events{/privacy}","received_events_url":"https://api.github.com/users/hongkai-dai/received_events","type":"User","site_admin":false},"created_at":"2016-11-28T22:14:04Z","updated_at":"2016-11-28T22:14:04Z","body":"\n\n\n\nReviewed 1 of 20 files at r1, 19 of 19 files at r3.\nReview status: all files reviewed at latest revision, 6 unresolved discussions.\n\n---\n\n*[robotiq_s_model_control/include/robotiq_s_model_control/s_model_api.h, line 74 at r3](https://reviewable.io:443/reviews/athackst/robotiq/1#-KXgwGF35O8bSUvX-Z2Y:-KXgwGF35O8bSUvX-Z2Z:b-a712ci) ([raw file](https://github.com/athackst/robotiq/blob/6b5fc963f7b130fb793a8a31ae9538e50434ddb7/robotiq_s_model_control/include/robotiq_s_model_control/s_model_api.h#L74)):*\n> ```objc\n>     void getRaw(SModelClientBase::GripperInput &raw);\n> \n>     void getCommandPos(double &posA, double &posB, double &posC, double &posS);\n> ```\n\nconst method? Same for all getters. Also according to google style guide, all reference should be const https://google.github.io/styleguide/cppguide.html#Reference_Arguments, So the function signature should be\n```\nvoid getCommandPos(double* posA, double* posB, double* posC, double* posS) const;\n```\n\n---\n\n*[robotiq_s_model_control/include/robotiq_s_model_control/s_model_api.h, line 110 at r3](https://reviewable.io:443/reviews/athackst/robotiq/1#-KXgxPjPqOnmb8XrJ5ZU:-KXgxPjPqOnmb8XrJ5ZV:b8bkb45) ([raw file](https://github.com/athackst/robotiq/blob/6b5fc963f7b130fb793a8a31ae9538e50434ddb7/robotiq_s_model_control/include/robotiq_s_model_control/s_model_api.h#L110)):*\n> ```objc\n>     value = value < std::numeric_limits<T>::min() ? std::numeric_limits<T>::min() : value;\n>     value = value > std::numeric_limits<T>::max() ? std::numeric_limits<T>::max() : value;\n>     return (T)value;\n> ```\n\nuse `static_cast<T>(value)` according to google style guide https://google.github.io/styleguide/cppguide.html#Casting\n\n---\n\n*[robotiq_s_model_control/include/robotiq_s_model_control/s_model_hw_interface.h, line 56 at r3](https://reviewable.io:443/reviews/athackst/robotiq/1#-KXgyGrZIF4znA8-uW5C:-KXgyGrZIF4znA8-uW5D:b-w7z99r) ([raw file](https://github.com/athackst/robotiq/blob/6b5fc963f7b130fb793a8a31ae9538e50434ddb7/robotiq_s_model_control/include/robotiq_s_model_control/s_model_hw_interface.h#L56)):*\n> ```objc\n>     std::vector<double> j_cmd_pos_;\n> \n>     boost::shared_ptr<SModelAPI> hw_driver_;\n> ```\n\nWhy using `boost::shared_ptr` instead of `std::shared_ptr`?\n\n---\n\n*[robotiq_s_model_control/src/robotiq_s_model_control/s_model_can_client.cpp, line 33 at r3](https://reviewable.io:443/reviews/athackst/robotiq/1#-KXgz00OU32CdSt2GrDM:-KXgz00Ps-yXC-zHahCU:b-y890yi) ([raw file](https://github.com/athackst/robotiq/blob/6b5fc963f7b130fb793a8a31ae9538e50434ddb7/robotiq_s_model_control/src/robotiq_s_model_control/s_model_can_client.cpp#L33)):*\n> ```cpp\n>     ,driver_(driver)\n> {\n>     resp_header_.id = can_id + 0x580;\n> ```\n\nDocument where `0x580` comes from?\n\n---\n\n*[robotiq_s_model_control/src/robotiq_s_model_control/s_model_can_client.cpp, line 211 at r3](https://reviewable.io:443/reviews/athackst/robotiq/1#-KXgzCaVFaaa86kC8mUX:-KXgzCaVFaaa86kC8mUY:b-k9ysid) ([raw file](https://github.com/athackst/robotiq/blob/6b5fc963f7b130fb793a8a31ae9538e50434ddb7/robotiq_s_model_control/src/robotiq_s_model_control/s_model_can_client.cpp#L211)):*\n> ```cpp\n>         can::Frame serial_request;\n>         serial_request.id = can_id_+0x600;\n>         serial_request.dlc = (unsigned char)8;\n> ```\n\n`static_cast<unsigned char>(8)`\n\n---\n\n*[robotiq_s_model_control/src/robotiq_s_model_control/s_model_can_client.cpp, line 215 at r3](https://reviewable.io:443/reviews/athackst/robotiq/1#-KXgzGOl2WtryEf9mjEB:-KXgzGOl2WtryEf9mjEC:b-k9yyfx) ([raw file](https://github.com/athackst/robotiq/blob/6b5fc963f7b130fb793a8a31ae9538e50434ddb7/robotiq_s_model_control/src/robotiq_s_model_control/s_model_can_client.cpp#L215)):*\n> ```cpp\n>         serial_request.is_rtr = 0;\n>         serial_request.is_extended = 0;\n>         serial_request.data.assign((unsigned char)0);\n> ```\n\n`static_cast<unsigned char>(0)`\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/athackst/robotiq/1)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"hongkai-dai"}}