{"action":"created","comment":{"url":"https://api.github.com/repos/3drobotics/rust-ptp/pulls/comments/70841428","id":70841428,"diff_hunk":"@@ -942,56 +928,70 @@ impl<'a> PtpCamera<'a> {\n             try!(self.handle.write_bulk(self.ep_out, &data_message, timeout));\n         }\n \n+        let tid = self.current_tid; // transaction id we're waiting for a response to\n         self.current_tid += 1;\n \n-        let mut data = None;\n+        // request phase is followed by data phase (optional) and response phase.\n+        // read both, check the status on the response, and return the data payload, if any.\n+        //\n+        // NB: responses with mismatching transaction IDs are discarded - does this represent\n+        //      an error, or should we do anything more helpful in this case?\n+        let mut data_phase_payload = vec![];\n         loop {\n-            unsafe {\n-                self.buf.set_len(0);\n-            }\n-\n-            loop {\n-                let chunk_size = 256 * 1024;\n-                let current_len = self.buf.len();\n-                let current_capacity = self.buf.capacity();\n-                if current_capacity - current_len < chunk_size {\n-                    self.buf.reserve(chunk_size);\n-                }\n-                let remaining_buf = unsafe {\n-                    slice::from_raw_parts_mut(self.buf.get_unchecked_mut(current_len) as *mut _, chunk_size)\n-                };\n-                let timespec = time::get_time();\n-                trace!(\"Read Data  [{}:{:09}] - length:{:?} remaining:{:?}\",\n-                       timespec.sec,\n-                       timespec.nsec,\n-                       current_len,\n-                       remaining_buf.len());\n-                let len = try!(self.handle.read_bulk(self.ep_in, remaining_buf, timeout));\n-                unsafe {\n-                    self.buf.set_len(current_len + len);\n-                }\n-                // debug!(\"new buf len [{:?}] into {:?}\", self.buf.len(), remaining_buf.len());\n-                if len < remaining_buf.len() {\n-                    break;\n+            let (container, payload) = try!(self.read_txn_phase());\n+            if container.belongs_to(tid) {\n+                match container.kind {\n+                    PtpContainerType::Data => {\n+                        data_phase_payload = payload;\n+                    },\n+                    PtpContainerType::Response => {\n+                        if container.code != StandardResponseCode::Ok {\n+                            return Err(Error::Response(container.code));\n+                        }\n+                        return Ok(data_phase_payload);\n+                    },\n+                    _ => {}\n                 }\n+            } else {\n+                debug!(\"mismatched txnid {}, expecting {}\", container.tid, tid);\n             }\n+        }\n+    }\n \n-            let (mtype, mut msg) = try!(PtpTransaction::parse(&self.buf));\n+    // helper for command() above, retrieve container info and payload for the current phase\n+    fn read_txn_phase(&mut self) -> Result<(PtpContainerInfo, Vec<u8>), Error> {\n+        // large enough to fetch objects other than media in a single read\n+        let mut buf = Vec::with_capacity(100 * 1024);\n+        let timeout = Duration::from_secs(2);\n \n-            if mtype == PtpContainerType::Data && msg.is_response(&transaction) {\n-                data = Some(msg.data);\n-            } else if mtype == PtpContainerType::Response && msg.is_response(&transaction) {\n-                if let Some(data) = data {\n-                    msg.data = data;\n-                }\n-                \n-                if msg.code != StandardResponseCode::Ok {\n-                    return Err(Error::Response(msg.code));\n-                }\n-                \n-                return Ok(msg.data);\n+        let bufslice = unsafe {\n+            slice::from_raw_parts_mut(buf.as_mut_ptr(), buf.capacity())\n+        };\n+        let n = try!(self.handle.read_bulk(self.ep_in, bufslice, timeout));\n+        unsafe { buf.set_len(n); }\n+\n+        let cinfo = try!(PtpContainerInfo::parse(&buf[..]));\n+        trace!(\"container {:?}, bulk rx {}\", cinfo, n);\n+\n+        // response didn't fit into our original buf? resize and read the rest\n+        // should ideally require a single read, but loop to handle short reads.","path":"src/lib.rs","position":191,"original_position":191,"commit_id":"994fe1b75d5aaca689400c6ba42541a56e0931da","original_commit_id":"994fe1b75d5aaca689400c6ba42541a56e0931da","user":{"login":"liamstask","id":51256,"avatar_url":"https://avatars.githubusercontent.com/u/51256?v=3","gravatar_id":"","url":"https://api.github.com/users/liamstask","html_url":"https://github.com/liamstask","followers_url":"https://api.github.com/users/liamstask/followers","following_url":"https://api.github.com/users/liamstask/following{/other_user}","gists_url":"https://api.github.com/users/liamstask/gists{/gist_id}","starred_url":"https://api.github.com/users/liamstask/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/liamstask/subscriptions","organizations_url":"https://api.github.com/users/liamstask/orgs","repos_url":"https://api.github.com/users/liamstask/repos","events_url":"https://api.github.com/users/liamstask/events{/privacy}","received_events_url":"https://api.github.com/users/liamstask/received_events","type":"User","site_admin":false},"body":"looks like a short packet should only be sent to indicate the end of a command phase, so could otherwise treat it as an error. will update.","created_at":"2016-07-14T16:52:52Z","updated_at":"2016-07-14T16:52:52Z","html_url":"https://github.com/3drobotics/rust-ptp/pull/8#discussion_r70841428","pull_request_url":"https://api.github.com/repos/3drobotics/rust-ptp/pulls/8","_links":{"self":{"href":"https://api.github.com/repos/3drobotics/rust-ptp/pulls/comments/70841428"},"html":{"href":"https://github.com/3drobotics/rust-ptp/pull/8#discussion_r70841428"},"pull_request":{"href":"https://api.github.com/repos/3drobotics/rust-ptp/pulls/8"}}},"pull_request":{"url":"https://api.github.com/repos/3drobotics/rust-ptp/pulls/8","id":77389816,"html_url":"https://github.com/3drobotics/rust-ptp/pull/8","diff_url":"https://github.com/3drobotics/rust-ptp/pull/8.diff","patch_url":"https://github.com/3drobotics/rust-ptp/pull/8.patch","issue_url":"https://api.github.com/repos/3drobotics/rust-ptp/issues/8","number":8,"state":"open","locked":false,"title":"command(): read larger chunks at a time, keep the bus active","user":{"login":"liamstask","id":51256,"avatar_url":"https://avatars.githubusercontent.com/u/51256?v=3","gravatar_id":"","url":"https://api.github.com/users/liamstask","html_url":"https://github.com/liamstask","followers_url":"https://api.github.com/users/liamstask/followers","following_url":"https://api.github.com/users/liamstask/following{/other_user}","gists_url":"https://api.github.com/users/liamstask/gists{/gist_id}","starred_url":"https://api.github.com/users/liamstask/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/liamstask/subscriptions","organizations_url":"https://api.github.com/users/liamstask/orgs","repos_url":"https://api.github.com/users/liamstask/repos","events_url":"https://api.github.com/users/liamstask/events{/privacy}","received_events_url":"https://api.github.com/users/liamstask/received_events","type":"User","site_admin":false},"body":"the response header tells us the payload length, so attempt to read it all in one chunk to avoid issuing several bulk read requests.\r\n\r\nalso, renames PtpTransaction to PtpContainerInfo since a) PtpTransaction was a misnomer and b) container terminology is unified with existing `PtpContainerType`","created_at":"2016-07-14T02:04:48Z","updated_at":"2016-07-14T16:52:52Z","closed_at":null,"merged_at":null,"merge_commit_sha":"b1551c34875b894ca2eff7057b3bab74763bb6d7","assignee":{"login":"kevinmehall","id":119951,"avatar_url":"https://avatars.githubusercontent.com/u/119951?v=3","gravatar_id":"","url":"https://api.github.com/users/kevinmehall","html_url":"https://github.com/kevinmehall","followers_url":"https://api.github.com/users/kevinmehall/followers","following_url":"https://api.github.com/users/kevinmehall/following{/other_user}","gists_url":"https://api.github.com/users/kevinmehall/gists{/gist_id}","starred_url":"https://api.github.com/users/kevinmehall/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kevinmehall/subscriptions","organizations_url":"https://api.github.com/users/kevinmehall/orgs","repos_url":"https://api.github.com/users/kevinmehall/repos","events_url":"https://api.github.com/users/kevinmehall/events{/privacy}","received_events_url":"https://api.github.com/users/kevinmehall/received_events","type":"User","site_admin":false},"assignees":[{"login":"kevinmehall","id":119951,"avatar_url":"https://avatars.githubusercontent.com/u/119951?v=3","gravatar_id":"","url":"https://api.github.com/users/kevinmehall","html_url":"https://github.com/kevinmehall","followers_url":"https://api.github.com/users/kevinmehall/followers","following_url":"https://api.github.com/users/kevinmehall/following{/other_user}","gists_url":"https://api.github.com/users/kevinmehall/gists{/gist_id}","starred_url":"https://api.github.com/users/kevinmehall/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kevinmehall/subscriptions","organizations_url":"https://api.github.com/users/kevinmehall/orgs","repos_url":"https://api.github.com/users/kevinmehall/repos","events_url":"https://api.github.com/users/kevinmehall/events{/privacy}","received_events_url":"https://api.github.com/users/kevinmehall/received_events","type":"User","site_admin":false}],"milestone":null,"commits_url":"https://api.github.com/repos/3drobotics/rust-ptp/pulls/8/commits","review_comments_url":"https://api.github.com/repos/3drobotics/rust-ptp/pulls/8/comments","review_comment_url":"https://api.github.com/repos/3drobotics/rust-ptp/pulls/comments{/number}","comments_url":"https://api.github.com/repos/3drobotics/rust-ptp/issues/8/comments","statuses_url":"https://api.github.com/repos/3drobotics/rust-ptp/statuses/994fe1b75d5aaca689400c6ba42541a56e0931da","head":{"label":"3drobotics:get-object-bulk","ref":"get-object-bulk","sha":"994fe1b75d5aaca689400c6ba42541a56e0931da","user":{"login":"3drobotics","id":857602,"avatar_url":"https://avatars.githubusercontent.com/u/857602?v=3","gravatar_id":"","url":"https://api.github.com/users/3drobotics","html_url":"https://github.com/3drobotics","followers_url":"https://api.github.com/users/3drobotics/followers","following_url":"https://api.github.com/users/3drobotics/following{/other_user}","gists_url":"https://api.github.com/users/3drobotics/gists{/gist_id}","starred_url":"https://api.github.com/users/3drobotics/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/3drobotics/subscriptions","organizations_url":"https://api.github.com/users/3drobotics/orgs","repos_url":"https://api.github.com/users/3drobotics/repos","events_url":"https://api.github.com/users/3drobotics/events{/privacy}","received_events_url":"https://api.github.com/users/3drobotics/received_events","type":"Organization","site_admin":false},"repo":{"id":54347970,"name":"rust-ptp","full_name":"3drobotics/rust-ptp","owner":{"login":"3drobotics","id":857602,"avatar_url":"https://avatars.githubusercontent.com/u/857602?v=3","gravatar_id":"","url":"https://api.github.com/users/3drobotics","html_url":"https://github.com/3drobotics","followers_url":"https://api.github.com/users/3drobotics/followers","following_url":"https://api.github.com/users/3drobotics/following{/other_user}","gists_url":"https://api.github.com/users/3drobotics/gists{/gist_id}","starred_url":"https://api.github.com/users/3drobotics/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/3drobotics/subscriptions","organizations_url":"https://api.github.com/users/3drobotics/orgs","repos_url":"https://api.github.com/users/3drobotics/repos","events_url":"https://api.github.com/users/3drobotics/events{/privacy}","received_events_url":"https://api.github.com/users/3drobotics/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/3drobotics/rust-ptp","description":"Picture Transfer Protocol driver in Rust.","fork":false,"url":"https://api.github.com/repos/3drobotics/rust-ptp","forks_url":"https://api.github.com/repos/3drobotics/rust-ptp/forks","keys_url":"https://api.github.com/repos/3drobotics/rust-ptp/keys{/key_id}","collaborators_url":"https://api.github.com/repos/3drobotics/rust-ptp/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/3drobotics/rust-ptp/teams","hooks_url":"https://api.github.com/repos/3drobotics/rust-ptp/hooks","issue_events_url":"https://api.github.com/repos/3drobotics/rust-ptp/issues/events{/number}","events_url":"https://api.github.com/repos/3drobotics/rust-ptp/events","assignees_url":"https://api.github.com/repos/3drobotics/rust-ptp/assignees{/user}","branches_url":"https://api.github.com/repos/3drobotics/rust-ptp/branches{/branch}","tags_url":"https://api.github.com/repos/3drobotics/rust-ptp/tags","blobs_url":"https://api.github.com/repos/3drobotics/rust-ptp/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/3drobotics/rust-ptp/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/3drobotics/rust-ptp/git/refs{/sha}","trees_url":"https://api.github.com/repos/3drobotics/rust-ptp/git/trees{/sha}","statuses_url":"https://api.github.com/repos/3drobotics/rust-ptp/statuses/{sha}","languages_url":"https://api.github.com/repos/3drobotics/rust-ptp/languages","stargazers_url":"https://api.github.com/repos/3drobotics/rust-ptp/stargazers","contributors_url":"https://api.github.com/repos/3drobotics/rust-ptp/contributors","subscribers_url":"https://api.github.com/repos/3drobotics/rust-ptp/subscribers","subscription_url":"https://api.github.com/repos/3drobotics/rust-ptp/subscription","commits_url":"https://api.github.com/repos/3drobotics/rust-ptp/commits{/sha}","git_commits_url":"https://api.github.com/repos/3drobotics/rust-ptp/git/commits{/sha}","comments_url":"https://api.github.com/repos/3drobotics/rust-ptp/comments{/number}","issue_comment_url":"https://api.github.com/repos/3drobotics/rust-ptp/issues/comments{/number}","contents_url":"https://api.github.com/repos/3drobotics/rust-ptp/contents/{+path}","compare_url":"https://api.github.com/repos/3drobotics/rust-ptp/compare/{base}...{head}","merges_url":"https://api.github.com/repos/3drobotics/rust-ptp/merges","archive_url":"https://api.github.com/repos/3drobotics/rust-ptp/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/3drobotics/rust-ptp/downloads","issues_url":"https://api.github.com/repos/3drobotics/rust-ptp/issues{/number}","pulls_url":"https://api.github.com/repos/3drobotics/rust-ptp/pulls{/number}","milestones_url":"https://api.github.com/repos/3drobotics/rust-ptp/milestones{/number}","notifications_url":"https://api.github.com/repos/3drobotics/rust-ptp/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/3drobotics/rust-ptp/labels{/name}","releases_url":"https://api.github.com/repos/3drobotics/rust-ptp/releases{/id}","deployments_url":"https://api.github.com/repos/3drobotics/rust-ptp/deployments","created_at":"2016-03-21T00:22:45Z","updated_at":"2016-06-07T20:31:46Z","pushed_at":"2016-07-14T02:04:48Z","git_url":"git://github.com/3drobotics/rust-ptp.git","ssh_url":"git@github.com:3drobotics/rust-ptp.git","clone_url":"https://github.com/3drobotics/rust-ptp.git","svn_url":"https://github.com/3drobotics/rust-ptp","homepage":"https://dronekit.github.io/rust-ptp/ptp/","size":2797,"stargazers_count":0,"watchers_count":0,"language":"Rust","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":0,"mirror_url":null,"open_issues_count":2,"forks":0,"open_issues":2,"watchers":0,"default_branch":"master"}},"base":{"label":"3drobotics:master","ref":"master","sha":"63aced894ff7c99adea17996b56d7d5e9542e2e9","user":{"login":"3drobotics","id":857602,"avatar_url":"https://avatars.githubusercontent.com/u/857602?v=3","gravatar_id":"","url":"https://api.github.com/users/3drobotics","html_url":"https://github.com/3drobotics","followers_url":"https://api.github.com/users/3drobotics/followers","following_url":"https://api.github.com/users/3drobotics/following{/other_user}","gists_url":"https://api.github.com/users/3drobotics/gists{/gist_id}","starred_url":"https://api.github.com/users/3drobotics/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/3drobotics/subscriptions","organizations_url":"https://api.github.com/users/3drobotics/orgs","repos_url":"https://api.github.com/users/3drobotics/repos","events_url":"https://api.github.com/users/3drobotics/events{/privacy}","received_events_url":"https://api.github.com/users/3drobotics/received_events","type":"Organization","site_admin":false},"repo":{"id":54347970,"name":"rust-ptp","full_name":"3drobotics/rust-ptp","owner":{"login":"3drobotics","id":857602,"avatar_url":"https://avatars.githubusercontent.com/u/857602?v=3","gravatar_id":"","url":"https://api.github.com/users/3drobotics","html_url":"https://github.com/3drobotics","followers_url":"https://api.github.com/users/3drobotics/followers","following_url":"https://api.github.com/users/3drobotics/following{/other_user}","gists_url":"https://api.github.com/users/3drobotics/gists{/gist_id}","starred_url":"https://api.github.com/users/3drobotics/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/3drobotics/subscriptions","organizations_url":"https://api.github.com/users/3drobotics/orgs","repos_url":"https://api.github.com/users/3drobotics/repos","events_url":"https://api.github.com/users/3drobotics/events{/privacy}","received_events_url":"https://api.github.com/users/3drobotics/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/3drobotics/rust-ptp","description":"Picture Transfer Protocol driver in Rust.","fork":false,"url":"https://api.github.com/repos/3drobotics/rust-ptp","forks_url":"https://api.github.com/repos/3drobotics/rust-ptp/forks","keys_url":"https://api.github.com/repos/3drobotics/rust-ptp/keys{/key_id}","collaborators_url":"https://api.github.com/repos/3drobotics/rust-ptp/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/3drobotics/rust-ptp/teams","hooks_url":"https://api.github.com/repos/3drobotics/rust-ptp/hooks","issue_events_url":"https://api.github.com/repos/3drobotics/rust-ptp/issues/events{/number}","events_url":"https://api.github.com/repos/3drobotics/rust-ptp/events","assignees_url":"https://api.github.com/repos/3drobotics/rust-ptp/assignees{/user}","branches_url":"https://api.github.com/repos/3drobotics/rust-ptp/branches{/branch}","tags_url":"https://api.github.com/repos/3drobotics/rust-ptp/tags","blobs_url":"https://api.github.com/repos/3drobotics/rust-ptp/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/3drobotics/rust-ptp/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/3drobotics/rust-ptp/git/refs{/sha}","trees_url":"https://api.github.com/repos/3drobotics/rust-ptp/git/trees{/sha}","statuses_url":"https://api.github.com/repos/3drobotics/rust-ptp/statuses/{sha}","languages_url":"https://api.github.com/repos/3drobotics/rust-ptp/languages","stargazers_url":"https://api.github.com/repos/3drobotics/rust-ptp/stargazers","contributors_url":"https://api.github.com/repos/3drobotics/rust-ptp/contributors","subscribers_url":"https://api.github.com/repos/3drobotics/rust-ptp/subscribers","subscription_url":"https://api.github.com/repos/3drobotics/rust-ptp/subscription","commits_url":"https://api.github.com/repos/3drobotics/rust-ptp/commits{/sha}","git_commits_url":"https://api.github.com/repos/3drobotics/rust-ptp/git/commits{/sha}","comments_url":"https://api.github.com/repos/3drobotics/rust-ptp/comments{/number}","issue_comment_url":"https://api.github.com/repos/3drobotics/rust-ptp/issues/comments{/number}","contents_url":"https://api.github.com/repos/3drobotics/rust-ptp/contents/{+path}","compare_url":"https://api.github.com/repos/3drobotics/rust-ptp/compare/{base}...{head}","merges_url":"https://api.github.com/repos/3drobotics/rust-ptp/merges","archive_url":"https://api.github.com/repos/3drobotics/rust-ptp/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/3drobotics/rust-ptp/downloads","issues_url":"https://api.github.com/repos/3drobotics/rust-ptp/issues{/number}","pulls_url":"https://api.github.com/repos/3drobotics/rust-ptp/pulls{/number}","milestones_url":"https://api.github.com/repos/3drobotics/rust-ptp/milestones{/number}","notifications_url":"https://api.github.com/repos/3drobotics/rust-ptp/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/3drobotics/rust-ptp/labels{/name}","releases_url":"https://api.github.com/repos/3drobotics/rust-ptp/releases{/id}","deployments_url":"https://api.github.com/repos/3drobotics/rust-ptp/deployments","created_at":"2016-03-21T00:22:45Z","updated_at":"2016-06-07T20:31:46Z","pushed_at":"2016-07-14T02:04:48Z","git_url":"git://github.com/3drobotics/rust-ptp.git","ssh_url":"git@github.com:3drobotics/rust-ptp.git","clone_url":"https://github.com/3drobotics/rust-ptp.git","svn_url":"https://github.com/3drobotics/rust-ptp","homepage":"https://dronekit.github.io/rust-ptp/ptp/","size":2797,"stargazers_count":0,"watchers_count":0,"language":"Rust","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":0,"mirror_url":null,"open_issues_count":2,"forks":0,"open_issues":2,"watchers":0,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/3drobotics/rust-ptp/pulls/8"},"html":{"href":"https://github.com/3drobotics/rust-ptp/pull/8"},"issue":{"href":"https://api.github.com/repos/3drobotics/rust-ptp/issues/8"},"comments":{"href":"https://api.github.com/repos/3drobotics/rust-ptp/issues/8/comments"},"review_comments":{"href":"https://api.github.com/repos/3drobotics/rust-ptp/pulls/8/comments"},"review_comment":{"href":"https://api.github.com/repos/3drobotics/rust-ptp/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/3drobotics/rust-ptp/pulls/8/commits"},"statuses":{"href":"https://api.github.com/repos/3drobotics/rust-ptp/statuses/994fe1b75d5aaca689400c6ba42541a56e0931da"}}}}
{"actor":{"display_login":"liamstask"}}