-
Notifications
You must be signed in to change notification settings - Fork 43
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
Add Graupner ESC telemetry support #174
Conversation
src/main/sensors/esc_sensor.c
Outdated
rrfsmMinFrameLength = GRAUPNER_MIN_FRAME_LENGTH; | ||
rrfsmAccept = graupnerAccept; | ||
rrfsmDecode = graupnerDecode; | ||
rrfsmCrank = NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rrfsmCrank is initialized to NULL + *SensorInit(...) is called once - no harm but initialization here not necessary
src/main/sensors/esc_sensor.c
Outdated
case ESC_SENSOR_PROTO_GRAUPNER: | ||
callback = graupnerSensorInit(); | ||
baudrate = 19200; | ||
options |= SERIAL_BIDIR; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SERIAL_BIDIR option is a function of config->halfDuplex and is already set if configured
... but this protocol appears to function only in request/response mode making half duplex (or full duplex w/ tx/rx) required.
Choices...
- force half duplex operation ON here (now port state does not match port config - may create issues elsewhere)
- enforce config->halfDuplex on setting config->protocol and fail the init (return false if not config->halfDuplex) as a sanity check? (a bit more effort but feels more robust)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had the second option first, but then I thought what's the point there are no choices here. Why do we want the user to play roulette with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The right thing is to set use the validateAndFixConfig routine and change the halfDuplex to true if Graupner is selected.
I'll fix this.
No description provided.