Skip to content

Commit 17f3db9

Browse files
niklautdakejahl
authored andcommitted
[serial] Fix byte size, flow control, parity, stop bits configuration
1 parent 25138d0 commit 17f3db9

File tree

3 files changed

+199
-39
lines changed

3 files changed

+199
-39
lines changed

platforms/nuttx/src/px4/common/SerialImpl.cpp

Lines changed: 80 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,34 @@ bool SerialImpl::configure()
180180
//
181181
uart_config.c_lflag &= ~(ECHO | ECHONL | ICANON | IEXTEN | ISIG);
182182

183-
/* no parity, one stop bit, disable flow control */
184-
uart_config.c_cflag &= ~(CSTOPB | PARENB | CRTSCTS);
183+
// Control modes
184+
uart_config.c_cflag = 0;
185+
186+
switch (_bytesize) {
187+
case ByteSize::FiveBits: uart_config.c_cflag |= CS5; break;
188+
189+
case ByteSize::SixBits: uart_config.c_cflag |= CS6; break;
190+
191+
case ByteSize::SevenBits: uart_config.c_cflag |= CS7; break;
192+
193+
case ByteSize::EightBits: uart_config.c_cflag |= CS8; break;
194+
}
195+
196+
if (_flowcontrol == FlowControl::Enabled) {
197+
uart_config.c_cflag |= CRTSCTS;
198+
}
199+
200+
if (_parity != Parity::None) {
201+
uart_config.c_cflag |= PARENB;
202+
}
203+
204+
if (_parity == Parity::Odd) {
205+
uart_config.c_cflag |= PARODD;
206+
}
207+
208+
if (_stopbits == StopBits::Two) {
209+
uart_config.c_cflag |= CSTOPB;
210+
}
185211

186212
/* set baud rate */
187213
if ((termios_state = cfsetispeed(&uart_config, speed)) < 0) {
@@ -486,7 +512,19 @@ ByteSize SerialImpl::getBytesize() const
486512

487513
bool SerialImpl::setBytesize(ByteSize bytesize)
488514
{
489-
return bytesize == ByteSize::EightBits;
515+
// check if already configured
516+
if ((bytesize == _bytesize) && _open) {
517+
return true;
518+
}
519+
520+
_bytesize = bytesize;
521+
522+
// process bytesize change now if port is already open
523+
if (_open) {
524+
return configure();
525+
}
526+
527+
return true;
490528
}
491529

492530
Parity SerialImpl::getParity() const
@@ -496,7 +534,19 @@ Parity SerialImpl::getParity() const
496534

497535
bool SerialImpl::setParity(Parity parity)
498536
{
499-
return parity == Parity::None;
537+
// check if already configured
538+
if ((parity == _parity) && _open) {
539+
return true;
540+
}
541+
542+
_parity = parity;
543+
544+
// process parity change now if port is already open
545+
if (_open) {
546+
return configure();
547+
}
548+
549+
return true;
500550
}
501551

502552
StopBits SerialImpl::getStopbits() const
@@ -506,7 +556,19 @@ StopBits SerialImpl::getStopbits() const
506556

507557
bool SerialImpl::setStopbits(StopBits stopbits)
508558
{
509-
return stopbits == StopBits::One;
559+
// check if already configured
560+
if ((stopbits == _stopbits) && _open) {
561+
return true;
562+
}
563+
564+
_stopbits = stopbits;
565+
566+
// process stopbits change now if port is already open
567+
if (_open) {
568+
return configure();
569+
}
570+
571+
return true;
510572
}
511573

512574
FlowControl SerialImpl::getFlowcontrol() const
@@ -516,7 +578,19 @@ FlowControl SerialImpl::getFlowcontrol() const
516578

517579
bool SerialImpl::setFlowcontrol(FlowControl flowcontrol)
518580
{
519-
return flowcontrol == FlowControl::Disabled;
581+
// check if already configured
582+
if ((flowcontrol == _flowcontrol) && _open) {
583+
return true;
584+
}
585+
586+
_flowcontrol = flowcontrol;
587+
588+
// process flowcontrol change now if port is already open
589+
if (_open) {
590+
return configure();
591+
}
592+
593+
return true;
520594
}
521595

522596
bool SerialImpl::getSingleWireMode() const

platforms/posix/src/px4/common/SerialImpl.cpp

Lines changed: 72 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,34 @@ bool SerialImpl::configure()
169169
//
170170
uart_config.c_lflag &= ~(ECHO | ECHONL | ICANON | IEXTEN | ISIG);
171171

172-
/* no parity, one stop bit, disable flow control */
173-
uart_config.c_cflag &= ~(CSTOPB | PARENB | CRTSCTS);
172+
// Control modes
173+
uart_config.c_cflag = 0;
174+
175+
switch (_bytesize) {
176+
case ByteSize::FiveBits: uart_config.c_cflag |= CS5; break;
177+
178+
case ByteSize::SixBits: uart_config.c_cflag |= CS6; break;
179+
180+
case ByteSize::SevenBits: uart_config.c_cflag |= CS7; break;
181+
182+
case ByteSize::EightBits: uart_config.c_cflag |= CS8; break;
183+
}
184+
185+
if (_flowcontrol == FlowControl::Enabled) {
186+
uart_config.c_cflag |= CRTSCTS;
187+
}
188+
189+
if (_parity != Parity::None) {
190+
uart_config.c_cflag |= PARENB;
191+
}
192+
193+
if (_parity == Parity::Odd) {
194+
uart_config.c_cflag |= PARODD;
195+
}
196+
197+
if (_stopbits == StopBits::Two) {
198+
uart_config.c_cflag |= CSTOPB;
199+
}
174200

175201
/* set baud rate */
176202
if ((termios_state = cfsetispeed(&uart_config, speed)) < 0) {
@@ -445,7 +471,6 @@ uint32_t SerialImpl::getBaudrate() const
445471

446472
bool SerialImpl::setBaudrate(uint32_t baudrate)
447473
{
448-
// check if already configured
449474
if ((baudrate == _baudrate) && _open) {
450475
return true;
451476
}
@@ -467,7 +492,17 @@ ByteSize SerialImpl::getBytesize() const
467492

468493
bool SerialImpl::setBytesize(ByteSize bytesize)
469494
{
470-
return bytesize == ByteSize::EightBits;
495+
if ((bytesize == _bytesize) && _open) {
496+
return true;
497+
}
498+
499+
_bytesize = bytesize;
500+
501+
if (_open) {
502+
return configure();
503+
}
504+
505+
return true;
471506
}
472507

473508
Parity SerialImpl::getParity() const
@@ -477,7 +512,17 @@ Parity SerialImpl::getParity() const
477512

478513
bool SerialImpl::setParity(Parity parity)
479514
{
480-
return parity == Parity::None;
515+
if ((parity == _parity) && _open) {
516+
return true;
517+
}
518+
519+
_parity = parity;
520+
521+
if (_open) {
522+
return configure();
523+
}
524+
525+
return true;
481526
}
482527

483528
StopBits SerialImpl::getStopbits() const
@@ -487,7 +532,17 @@ StopBits SerialImpl::getStopbits() const
487532

488533
bool SerialImpl::setStopbits(StopBits stopbits)
489534
{
490-
return stopbits == StopBits::One;
535+
if ((stopbits == _stopbits) && _open) {
536+
return true;
537+
}
538+
539+
_stopbits = stopbits;
540+
541+
if (_open) {
542+
return configure();
543+
}
544+
545+
return true;
491546
}
492547

493548
FlowControl SerialImpl::getFlowcontrol() const
@@ -497,7 +552,17 @@ FlowControl SerialImpl::getFlowcontrol() const
497552

498553
bool SerialImpl::setFlowcontrol(FlowControl flowcontrol)
499554
{
500-
return flowcontrol == FlowControl::Disabled;
555+
if ((flowcontrol == _flowcontrol) && _open) {
556+
return true;
557+
}
558+
559+
_flowcontrol = flowcontrol;
560+
561+
if (_open) {
562+
return configure();
563+
}
564+
565+
return true;
501566
}
502567

503568
bool SerialImpl::getSingleWireMode() const

platforms/qurt/src/px4/SerialImpl.cpp

Lines changed: 47 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -103,25 +103,14 @@ bool SerialImpl::open()
103103
return false;
104104
}
105105

106-
if (_bytesize != ByteSize::EightBits) {
107-
PX4_ERR("Qurt platform only supports ByteSize::EightBits");
108-
return false;
109-
}
106+
// Check all non-supported configurations without duplicating the error strings
107+
if (!setBytesize(_bytesize)) { return false; }
110108

111-
if (_parity != Parity::None) {
112-
PX4_ERR("Qurt platform only supports Parity::None");
113-
return false;
114-
}
109+
if (!setParity(_parity)) { return false; }
115110

116-
if (_stopbits != StopBits::One) {
117-
PX4_ERR("Qurt platform only supports StopBits::One");
118-
return false;
119-
}
111+
if (!setStopbits(_stopbits)) { return false; }
120112

121-
if (_flowcontrol != FlowControl::Disabled) {
122-
PX4_ERR("Qurt platform only supports FlowControl::Disabled");
123-
return false;
124-
}
113+
if (!setFlowcontrol(_flowcontrol)) { return false; }
125114

126115
if (!validatePort(_port)) {
127116
PX4_ERR("Invalid port %s", _port);
@@ -348,7 +337,12 @@ ByteSize SerialImpl::getBytesize() const
348337

349338
bool SerialImpl::setBytesize(ByteSize bytesize)
350339
{
351-
return bytesize == ByteSize::EightBits;
340+
if (bytesize != ByteSize::EightBits) {
341+
PX4_ERR("Qurt platform only supports ByteSize::EightBits");
342+
return false;
343+
}
344+
345+
return true;
352346
}
353347

354348
Parity SerialImpl::getParity() const
@@ -358,7 +352,12 @@ Parity SerialImpl::getParity() const
358352

359353
bool SerialImpl::setParity(Parity parity)
360354
{
361-
return parity == Parity::None;
355+
if (parity != Parity::None) {
356+
PX4_ERR("Qurt platform only supports Parity::None");
357+
return false;
358+
}
359+
360+
return true;
362361
}
363362

364363
StopBits SerialImpl::getStopbits() const
@@ -368,7 +367,12 @@ StopBits SerialImpl::getStopbits() const
368367

369368
bool SerialImpl::setStopbits(StopBits stopbits)
370369
{
371-
return stopbits == StopBits::One;
370+
if (stopbits != StopBits::One) {
371+
PX4_ERR("Qurt platform only supports StopBits::One");
372+
return false;
373+
}
374+
375+
return true;
372376
}
373377

374378
FlowControl SerialImpl::getFlowcontrol() const
@@ -378,7 +382,12 @@ FlowControl SerialImpl::getFlowcontrol() const
378382

379383
bool SerialImpl::setFlowcontrol(FlowControl flowcontrol)
380384
{
381-
return flowcontrol == FlowControl::Disabled;
385+
if (flowcontrol != FlowControl::Disabled) {
386+
PX4_ERR("Qurt platform only supports FlowControl::Disabled");
387+
return false;
388+
}
389+
390+
return true;
382391
}
383392

384393
bool SerialImpl::getSingleWireMode() const
@@ -388,8 +397,12 @@ bool SerialImpl::getSingleWireMode() const
388397

389398
bool SerialImpl::setSingleWireMode()
390399
{
391-
// Qurt platform does not support single wire mode
392-
return false;
400+
if (enable) {
401+
PX4_ERR("Qurt platform does not support single wire mode");
402+
return false;
403+
}
404+
405+
return true;
393406
}
394407

395408
bool SerialImpl::getSwapRxTxMode() const
@@ -399,14 +412,22 @@ bool SerialImpl::getSwapRxTxMode() const
399412

400413
bool SerialImpl::setSwapRxTxMode()
401414
{
402-
// Qurt platform does not support swap rx tx mode
403-
return false;
415+
if (enable) {
416+
PX4_ERR("Qurt platform does not support swap rx tx mode");
417+
return false;
418+
}
419+
420+
return true;
404421
}
405422

406423
bool SerialImpl::setInvertedMode(bool enable)
407424
{
408-
// Qurt platform does not support inverted mode
409-
return false == enable;
425+
if (enable) {
426+
PX4_ERR("Qurt platform does not support inverted mode");
427+
return false;
428+
}
429+
430+
return true;
410431
}
411432
bool SerialImpl::getInvertedMode() const
412433
{

0 commit comments

Comments
 (0)