Minor fixes for ofSerial on Windows #1139

Closed
wants to merge 6 commits into
from

Projects

None yet

3 participants

@prettyextreme

Fixes issues #1136 and #1137 in ofSerial.

Initializes bInited = false in the constructor (1136) and uses SetupDiGetDeviceRegistryPropertyA() in enumerateWin32Ports() to avoid WCHAR strings. (1137)

Tested with VS2010 in debug mode. No longer crashes on application exit when no serial device connected, and now properly enumerates serial ports on windows machine.

Josh Silverman added some commits Apr 4, 2012
Josh Silverman Fixes #1136 in which ofSerial::enumerateWin32Ports() would resolve
SetupDiGetDeviceRegistryProperty() to the --W() version with UNICODE
defined, later breaking the rest of the function. I have now hard-coded to
the --W() version of this function call.
b154009
Josh Silverman Fixes #1137 to initialize ofSerial.bInited = false in the constructor. 245f386
Josh Silverman Fixes #1137 to initialize ofSerial.bInited = false in the constructor. 2deefe8
@joshuajnoble
Contributor

This looks good to me, I'm just going to wait for one of the core members to merge it. Thanks for the patch :)

@prettyextreme

I've also added a commit that fixes a really obvious bug that was forcing a call to ofTexture.allocate on every call to ofImage.update(). This was seriously affecting the performance of my frame sequence player!

Sorry - I think this probably should have gone in its own pull-request. I'm still trying to get the hang of the git thing!

@joshuajnoble
Contributor

Yeah, different functionality should have different PRs. Thanks for finding all these bug tho, it's great.

@bilderbuchi
Member

@prettyextreme You have several branches on your machine: master, develop, and one branch for every pull request you have open. You create a branch where you do/put all the work for a particular pull request. When you update/correct stuff in that PR, you do it by committing to that branch. only after the PR is merged, you can safely delete the branch.
I don't know what will happen if you delete a branch before the PR is merged, but it can't be good, since the branch is what the PR uses for calculating the details of the PR.

I should be keeping temporary branches around for weeks until a Pull Request is accepted.

yes this is exactly what you should do. Coming from other VCS', this may be unusual, but git lives and breathes branches, they're a totally normal thing to have.

Missing PR: maybe it got closed? You can see all your PRs by clicking on "Yours" on the PR page. Then you have to click the "Closed" button to also list those which are closed/merged already. https://github.com/openframeworks/openFrameworks/pulls/prettyextreme

@prettyextreme

Thanks for the pointers. I think I'm finally all pulled, merged and cleaned
up. Even having read those OF workflow docs and links, it's really helpful
to hear the details of how people actually manage their own workflow.


Josh Silverman

On Tue, Apr 10, 2012 at 5:26 AM, Christoph Buchner <
reply@reply.github.com

wrote:

You have several branches on your machine: master, develop, and one
branch for every pull request you have open. You create a branch where you
do/put all the work for a particular pull request. When you update/correct
stuff in that PR, you do it by committing to that branch. only after the PR
is merged, you can safely delete the branch.
I don't know what will happen if you delete a branch before the PR is
merged, but it can't be good, since the branch is what the PR uses for
calculating the details of the PR.

I should be keeping temporary branches around for weeks until a Pull
Request is accepted.

yes this is exactly what you should do. Coming from other VCS', this may
be unusual, but git lives and breathes branches, they're a totally normal
thing to have.

Missing PR: maybe it got closed? You can see all your PRs by clicking on
"Yours" on the PR page. Then you have to click the "Closed" button to also
list those which are closed/merged already.
https://github.com/openframeworks/openFrameworks/pulls/prettyextreme


Reply to this email directly or view it on GitHub:

#1139 (comment)

@bilderbuchi
Member

you're welcome, no problem. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment