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 C++ Example with ported CheesyVisionServer #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

DjScribbles
Copy link

As noted on Chief Delphi, I did encounter some problems on our second day of using the ported CheesyVisionServer code, however other teams have used it successfully without any trouble (as did we our first day).

@DjScribbles
Copy link
Author

I know Wait() takes a double value of seconds, and in my experience (which is not in vxWorks) Sleep() takes an integer number of milliseconds, however it is ambiguous as near as I can tell, and could explain the odd behavior I've seen.

It may be clearer to change it to Wait(.05);

@AustinSchuh
Copy link
Contributor

I found the docs. sleep is in seconds.
http://www.vxdev.com/docs/vx55man/vxworks/ref/timerLib.html#sleep

On Thu, Apr 17, 2014 at 4:37 PM, DjScribbles [email protected]:

I know Wait() takes a double value of seconds, and in my experience (which
is not in vxWorks) Sleep() takes an integer number of milliseconds, however
it is ambiguous as near as I can tell, and could explain the odd behavior
I've seen.

It may be clearer to change it to Wait(.05);

Reply to this email directly or view it on GitHubhttps://github.com//pull/1#issuecomment-40774418
.

@DjScribbles
Copy link
Author

Ok, well excellent catch, would you like me to update and submit a new request, or would you like to resolve it on your end?

@tombot
Copy link
Member

tombot commented Apr 17, 2014

Just add another commit that fixes it. If you push to this branch it should
just work.
On Apr 17, 2014 4:45 PM, "DjScribbles" [email protected] wrote:

Ok, well excellent catch, would you like me to update and submit a new
request, or would you like to resolve it on your end?


Reply to this email directly or view it on GitHubhttps://github.com//pull/1#issuecomment-40774878
.

Sleep is in seconds, not milliseconds. To keep same resolution, switched
to Wait(double seconds).
@DjScribbles
Copy link
Author

Ok, done.


class CheesyVisionRobot : public IterativeRobot
{
CheesyVisionServer *server;
Copy link
Contributor

Choose a reason for hiding this comment

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

Your indentation is inconsistent...

Please explicitly make private members private.

class {
public:
...

private:
...
};

@AustinSchuh
Copy link
Contributor

Thanks for the pull request! We have a high quality bar (as you would expect), hence the large number of comments. Hopefully we can whip everything into shape quickly.

@DjScribbles
Copy link
Author

I don't want to sound rude, because I don't disagree with any of your feedback, but I don't really have the energy this week to follow through with them; we've had 3 weeks straight of back to back competitions; our season is now over, and I'm trying to get back to the rest of life for a while.

You are welcome to make any changes you wish, I'll make some quick comments above, but beyond that, it may be too late before I have the time and energy to go through everything you've suggested.

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.

4 participants