SNS: Serial Communication #120
Conversation
Codecov Report
@@ Coverage Diff @@
## master #120 +/- ##
=======================================
Coverage 33.95% 33.95%
=======================================
Files 67 67
Lines 3549 3549
=======================================
Hits 1205 1205
Misses 2344 2344 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Even if you hadn't violated every single rule in our style guide, I would still be advising you to rewrite all of this. I'm not sure this is good C code, but it's definitely not good C++.
You are using malloc, plenty of raw pointers, int as file descriptors, and many other things that don't have a place here. To solve the problem in question, I advise you to have a look at the following:
| // The payload that will be sent to the other device | ||
| struct Payload { | ||
| int16_t wheelEncoderA; | ||
| int16_t wheelEncoderB; |
|
|
||
| void SerialProtocol::configureTermios() | ||
| { | ||
| serial_ = open(serial_device_.c_str(), O_RDWR | O_NOCTTY | O_NDELAY); |
There was a problem hiding this comment.
I think some of these lines need comments as the constant names are not obvious (e.g. O_NOCTTY)
|
|
||
| struct termios options; | ||
| tcgetattr(serial_, &options); | ||
| options.c_cflag = CS8 | CLOCAL | CREAD; |
| #include <StreamSerialProtocol.h> | ||
| #include <ArduinoSerialProtocol.h> | ||
|
|
||
| int IRSensor1 = 2; |
There was a problem hiding this comment.
This seems brittle if we add/remove the number of IR sensors and wheel encoders? Can we not maintain a map of IR sensors and wheel encoders and loop through?
There was a problem hiding this comment.
I disagree. We are only ever going to have four IR sensors. Let's not make it more complicated than it needs to be.
Looking at it again, we should not be using a map, I stand by that. However, I don't see why we don't have something like
#define kNumWheelEncoders 4
const int kWheelEncoders[kNumWheelEncoders] = { /* ... */};Sorry, I was thinking in terms of C++.
Of course, this also raises the question of how we are going to do testing when we require all four to be present.
Co-authored-by: Arjun Naha <[email protected]>
Description
Implements UART communication with the Arduino to receive structs containing wheel encoder data.
Implemented