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

added partial decoding of serial communication to main.cpp #1

Closed
wants to merge 2 commits into from

Conversation

Qualot
Copy link

@Qualot Qualot commented Dec 19, 2014

modified from read(...FULL LENGH) to read(...PARTIAL) to read serial data smoothly.

@k-okada
Copy link
Member

k-okada commented Dec 19, 2014

please separate PR into two parts, one is just to change coding style, and the others is to change code logic.

@Qualot
Copy link
Author

Qualot commented Dec 20, 2014

Thanks, Okada-sensei,

Actually, I do not distinguish coding style and logic.
Could you tell me that or show me an example or reference?

Thank you,

                                    Shinsuke

2014-12-20 1:37 GMT+09:00 Kei Okada notifications@github.com:

please separate PR into two parts, one is just to change coding style, and
the others is to change code logic.


Reply to this email directly or view it on GitHub
#1 (comment).


中島慎介 Nakashima Shinsuke

東京大学 情報理工学系研究科 知能機械情報学専攻
情報システム工学研究室
Graduate School of Information Science and Technology
The University of Tokyo
Mechano-Informatics
JSK Laboratory

7-3-1 Hongo, Bunkyo-ku, Tokyo 113-8656 Japan
Tel: 03-5841-7416 / Fax: 03-5841-6285
Email: snakashima@jsk.imi.i.u-tokyo.ac.jp


@@ -1,67 +1,89 @@
#include <ros/ros.h>
#include <geometry_msgs/WrenchStamped.h>

#include <stdio.h>
Copy link
Member

Choose a reason for hiding this comment

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

+#include <stdio.h>
- #include  <stdio.h>

in this change, you just changed the number of white space, and there are no change in logically.

@130s
Copy link
Contributor

130s commented Dec 22, 2014

@Qualot For the case like you who's attentive to coding style, I recommend to separate commits like in the following order; (1st commit) edit the style, (2nd and later) add the actual change you'd like.

@Qualot
Copy link
Author

Qualot commented Dec 23, 2014

how about this pull request?

@130s
Copy link
Contributor

130s commented Dec 23, 2014

+1

@130s 130s mentioned this pull request Dec 23, 2014
over :
return (n);
}
return (n);
Copy link
Member

Choose a reason for hiding this comment

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

I think you still have diffs which is not related to the problem

k-okada referenced this pull request in k-okada/dynpick_driver Dec 28, 2014
k-okada referenced this pull request in k-okada/dynpick_driver Dec 28, 2014
@k-okada
Copy link
Member

k-okada commented Dec 28, 2014

ok, i created new PR #7 for typo and #8, #9 for read data
#8 and #9 is just a different implementation, so please choose one which you like, I haven't tested on real hardware, so please confirm this solves you problem > @Qualot

@130s
Copy link
Contributor

130s commented Dec 29, 2014

I confirmed both #8 and #9 "runs" with the device.
Since I still don't understand the problem this PR tries to address, I have no idea to pick either PR. @Qualot ?

(If I'm asked, I can only follow the general rule of thumb to avoid using goto so pick #9 that keeps away from goto. But I'm no C expert).

@k-okada
Copy link
Member

k-okada commented Jan 4, 2015

this is very common case when reading fixed byte from input stream, the read stream can take a number of bytes to read in its arguments, but it did not grantee that the read reads the given bytes, it reads possible bytes and returns the byte.
read() attempts to read up to count bytes from file descriptor fd into the buffer starting at buf.

@130s
Copy link
Contributor

130s commented Mar 22, 2015

Please choose either #8 or #9. I've tested with the real device, both ran but had no idea how I can verify the accuracy.

@130s 130s closed this Mar 22, 2015
@k-okada
Copy link
Member

k-okada commented Mar 24, 2015

This is just a code style difference, please choose either you prefer.

◉ Kei Okada

On Mon, Mar 23, 2015 at 12:24 AM, Isaac I.Y. Saito <notifications@github.com

wrote:

Please choose either #8 #8
or #9 #9. I've tested with
the real device, both ran but had no idea how I can verify the accuracy.


Reply to this email directly or view it on GitHub
#1 (comment).

130s added a commit that referenced this pull request Apr 1, 2015
fix for #1, do while to read enough data
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