search

OSVR / OSVR-HDK-MCU-Firmware

Firmware source for the main Atmel microcontroller in HMDs related to the OSVR HDK
13 stars 6 forks source link

CDC improvements #8

Closed rpavlik closed 7 years ago

rpavlik commented 7 years ago

Branch containing orthogonal improvements cherry-picked from my hdk2 dev branch, which improve USB behavior.

Notably, it resolves #7 - tested locally on a 1.2 by repeatedly plugging/unplugging HDMI in extended mode, with no serial console connected.

Note that for OSVR-Control's serial console to work with this PR (specifically the fix for #7) merged, https://github.com/OSVR/OSVR-Control/pull/16 must be merged.

godbyk commented 7 years ago

A couple general questions:

  1. What version of C is this targeting? Do we really need to use void parameters and to define variables at the tops of functions (and outside of for loop initializer clauses)?
  2. Can we fix the definition of CR while we're at it? I'd suggest just using '\n' and letting the platform define it. Otherwise, I'm cool with any of the standard line endings (CR, LF, CR+LF). There's no need to invent our own.
rpavlik commented 7 years ago

C99 (gnu99) - see makefiles - no need for variables at the top of scopes, but with the strict prototypes (?) flag gcc does whine at me if a function proto or inline definition had empty parameter list instead of void.

I also find the CR definition weird (had to write special parsing code to deal with it in Python and read a line at a time), but again, didn't touch working code. I don't know why that end of line convention was chosen in the first place, if there was a reason or if other code now depends on it.

rpavlik commented 7 years ago

(oh and actually for serial terminals it does work best to have the two byte end of line, i think, since they're often interpreted literally, I've found - new line, carriage return)

godbyk commented 7 years ago

Yes, usually CR+LF is used. LF+CR is very rare and certainly unexpected. (See here for some background information.)

Also note that \n and \r may not behave as you expect in C/C++. Their behavior varies across platforms and across modes (ASCII vs. binary). If you want to be explicit about CR and LF, I'd recommend using the ASCII codes directly (0x0d and 0x0a, respectively).

rpavlik commented 7 years ago

Didn't occur to me that \n might not be equivalent to ascii LF - weird! - though I am aware of the differences between opening in binary vs text mode. I'll handle that in a separate pr.