Skip to content

Fix for compatibility with other compilers/environments#45

Closed
suzuryo3893 wants to merge 2 commits intoAutonomyLab:masterfrom
suzuryo3893:origin_dev
Closed

Fix for compatibility with other compilers/environments#45
suzuryo3893 wants to merge 2 commits intoAutonomyLab:masterfrom
suzuryo3893:origin_dev

Conversation

@suzuryo3893
Copy link
Contributor

  • An initialization of a variable required. An undefined value results doing nothing in a switch-case statement.
  • Explicit settings for all options of serial communication including stop bits, character size, and parity are required.

Otherwize, it can be happened that Operation "7" (0x07) turns to be "135" (0x87)
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

Thanks for the patch!

I have a few nitpicks.

using namespace boost::asio;
port.open(portName);
port.set_option(serial_port::baud_rate(baud));
port.set_option(serial_port::baud_rate(baud));
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: use spaces instead of tabs

namespace create {

SerialStream::SerialStream(boost::shared_ptr<Data> d, const uint8_t& header) : Serial(d), headerByte(header){
SerialStream::SerialStream(boost::shared_ptr<Data> d, const uint8_t& header) : Serial(d), headerByte(header), readState(READ_HEADER){
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: Better to initialize readState before headerByte so they are in the order they are defined in the class.

@jacobperron jacobperron mentioned this pull request Aug 27, 2019
@jacobperron
Copy link
Member

I've addressed my own nitpicks in #50. Thanks again!

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.

2 participants