Skip to content

Commit 5aef0e4

Browse files
committed
Update post review.
Change setConfig to accept individual arguments. Change constructors to use {} for member variable initializers rather than (). In copy assignment of sfTkArdUART and sfTkArdUARTBus, add a check that doesn't self-reassign. In sfTkArdUARTBus, change raw pointers to `std::unique_ptr` to make clean up easier. Modify `test_uart.ino` to match new `setConfig` format.
1 parent 4351ac0 commit 5aef0e4

File tree

4 files changed

+104
-79
lines changed

4 files changed

+104
-79
lines changed

src/sfTk/sfTkIUART.h

Lines changed: 45 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,14 @@ typedef enum _sfTkUARTParity
2929
} sfTkUARTParity_t;
3030

3131
inline const char* parityToString(sfTkUARTParity_t parity) {
32-
switch(parity) {
33-
case kUARTParityEven: return "Even";
34-
case kUARTParityOdd: return "Odd";
35-
case kUARTParityNone: return "None";
36-
case kUARTParityMark: return "Mark";
37-
case kUARTParitySpace: return "Space";
38-
default: return "Unknown";
39-
}
32+
static const char* parityArray[] = {"Even", "Odd", "None", "Mark", "Space"};
33+
34+
// if the parity is out of bounds, return "Unknown"
35+
if(parity < kUARTParityEven || parity > kUARTParitySpace)
36+
return "Unknown";
37+
38+
// return the parity string
39+
return parityArray[((uint8_t)parity) - 1];
4040
}
4141

4242
typedef enum _sfTkUARTStopBits
@@ -47,12 +47,14 @@ typedef enum _sfTkUARTStopBits
4747
} sfTkUARTStopBits_t;
4848

4949
inline const char* stopBitsToString(sfTkUARTStopBits_t stopBits) {
50-
switch(stopBits) {
51-
case kUARTStopBitsOne: return "One";
52-
case kUARTStopBitsOneAndHalf: return "OneAndHalf";
53-
case kUARTStopBitsTwo: return "Two";
54-
default: return "Unknown";
55-
}
50+
static const char* stopBitsArray[] = {"One", "OneAndHalf", "Two"};
51+
52+
// Return "Unknown" if index is out of bounds (less than 0 or greater than 2)
53+
if (stopBits < kUARTStopBitsOne || stopBits > kUARTStopBitsTwo)
54+
return "Unknown";
55+
56+
// Return the stop bits string
57+
return stopBitsArray[(((uint8_t)stopBits) >> 4) - 1];
5658
}
5759

5860
typedef enum _sfTkUARTDataBits
@@ -64,13 +66,14 @@ typedef enum _sfTkUARTDataBits
6466
} sfTkUARTDataBits_t;
6567

6668
inline const uint8_t dataBitsToValue(sfTkUARTDataBits_t dataBits) {
67-
switch(dataBits) {
68-
case kUARTDataBitsFive: return 5;
69-
case kUARTDataBitsSix: return 6;
70-
case kUARTDataBitsSeven: return 7;
71-
case kUARTDataBitsEight: return 8;
72-
default: return 0; // Invalid data bits
73-
}
69+
static const uint8_t dataBitsArray[] = {5, 6, 7, 8};
70+
71+
// Check if data bits are within valid range
72+
if (dataBits < kUARTDataBitsFive || dataBits > kUARTDataBitsEight)
73+
return 0;
74+
75+
// Extract index using bit shift (removing first 8 bits) and subtract 1
76+
return dataBitsArray[(((uint16_t)dataBits) >> 8) - 1];
7477
}
7578

7679
typedef struct _sfTkUARTConfig
@@ -85,28 +88,12 @@ class sfTkIUART : public sfTkISerial
8588
{
8689
public:
8790
/**
88-
* @brief Constructor for the UART bus
89-
*
90-
*/
91-
sfTkIUART(void)
92-
{
93-
_config.baudRate = kDefaultBaudRate;
94-
_config.parity = kUARTParityNone;
95-
_config.stopBits = kUARTStopBitsOne;
96-
_config.dataBits = kUARTDataBitsEight;
97-
}
98-
99-
/**
100-
* @brief Constructor for the UART bus with the baud rate passed in
91+
* @brief Default constructor for the UART bus
10192
*
10293
* @param baudRate
10394
*/
104-
sfTkIUART(uint32_t baudRate)
95+
sfTkIUART(uint32_t baudRate = kDefaultBaudRate) : _config{kDefaultBaudRate, kUARTDataBitsEight, kUARTParityNone, kUARTStopBitsOne}
10596
{
106-
_config.baudRate = baudRate;
107-
_config.parity = kUARTParityNone;
108-
_config.stopBits = kUARTStopBitsOne;
109-
_config.dataBits = kUARTDataBitsEight;
11097
}
11198

11299
/**
@@ -217,16 +204,28 @@ class sfTkIUART : public sfTkISerial
217204
/**
218205
* @brief setter for the internal config object
219206
*
220-
* @param config The config struct to set
207+
* @param baudRate The baud rate to set
208+
* @param dataBits The data bits to set
209+
* @param parity The parity to set
210+
* @param stopBits The stop bits to set
221211
* @return sfTkError_t Returns ksfTkErrOk on success, or ksfTkErrFail code
222212
*/
223-
virtual sfTkError_t setConfig(const sfTkUARTConfig_t config)
213+
virtual sfTkError_t setConfig(const uint32_t baudRate = kDefaultBaudRate,
214+
const sfTkUARTDataBits_t dataBits = kDefaultDataBits,
215+
const sfTkUARTParity_t parity = kDefaultParity,
216+
const sfTkUARTStopBits_t stopBits = kDefaultStopBits)
224217
{
225-
if(_config.baudRate != config.baudRate ||
226-
_config.stopBits != config.stopBits ||
227-
_config.parity != config.parity ||
228-
_config.dataBits != config.dataBits)
229-
_config = config;
218+
if(_config.baudRate != baudRate)
219+
_config.baudRate = baudRate;
220+
221+
if(_config.dataBits != dataBits)
222+
_config.dataBits = dataBits;
223+
224+
if(_config.parity != parity)
225+
_config.parity = parity;
226+
227+
if(_config.stopBits != stopBits)
228+
_config.stopBits = stopBits;
230229

231230
return ksfTkErrOk;
232231
}

src/sfTkArdUART.cpp

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -152,13 +152,22 @@ sfTkError_t sfTkArdUART::setDataBits(const sfTkUARTDataBits_t dataBits)
152152
return _start(); // start the port again
153153
}
154154

155-
sfTkError_t sfTkArdUART::setConfig(const sfTkUARTConfig_t config)
155+
sfTkError_t sfTkArdUART::setConfig(const uint32_t baudRate,
156+
const sfTkUARTDataBits_t dataBits,
157+
const sfTkUARTParity_t parity,
158+
const sfTkUARTStopBits_t stopBits)
156159
{
157-
if(_config.baudRate != config.baudRate ||
158-
_config.stopBits != config.stopBits ||
159-
_config.parity != config.parity ||
160-
_config.dataBits != config.dataBits)
161-
_config = config;
160+
if(_config.baudRate != baudRate)
161+
_config.baudRate = baudRate;
162+
163+
if(_config.dataBits != dataBits)
164+
_config.dataBits = dataBits;
165+
166+
if(_config.parity != parity)
167+
_config.parity = parity;
168+
169+
if(_config.stopBits != stopBits)
170+
_config.stopBits = stopBits;
162171

163172
return _start(); // start the port again
164173
}

src/sfTkArdUART.h

Lines changed: 42 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@
1313

1414
#pragma once
1515

16+
#include <cstddef>
17+
#include <memory>
18+
1619
#include <Arduino.h>
1720
#include <api/HardwareSerial.h>
1821
#include <api/Print.h>
@@ -23,7 +26,6 @@
2326
#include "sfTk/sfTkError.h"
2427
#include "sfTk/sfTkISerial.h"
2528
#include "sfTkArduino.h"
26-
#include <cstddef>
2729
#include <sfTk/sfTkIUART.h>
2830
#include <sfTk/sfTkISerialBus.h>
2931

@@ -38,7 +40,7 @@ class sfTkArdUART : public sfTkIUART
3840
/**
3941
* @brief Constructor
4042
*/
41-
sfTkArdUART(void) : sfTkIUART(), _hwSerial(nullptr)
43+
sfTkArdUART(void) : sfTkIUART(), _hwSerial{nullptr}
4244
{
4345
}
4446

@@ -47,7 +49,7 @@ class sfTkArdUART : public sfTkIUART
4749
*
4850
* @param baudRate The baud rate to set
4951
*/
50-
sfTkArdUART(uint32_t baudRate) : sfTkIUART(baudRate), _hwSerial(nullptr)
52+
sfTkArdUART(uint32_t baudRate) : sfTkIUART(baudRate), _hwSerial{nullptr}
5153
{
5254
}
5355

@@ -56,7 +58,7 @@ class sfTkArdUART : public sfTkIUART
5658
*
5759
* @param config The UART configuration settings.
5860
*/
59-
sfTkArdUART(sfTkUARTConfig_t config) : sfTkIUART(config), _hwSerial(nullptr)
61+
sfTkArdUART(sfTkUARTConfig_t config) : sfTkIUART(config), _hwSerial{nullptr}
6062
{
6163
}
6264

@@ -65,14 +67,14 @@ class sfTkArdUART : public sfTkIUART
6567
*
6668
* @param uartPort Port for UART communication.
6769
*/
68-
sfTkArdUART(arduino::HardwareSerial &hwSerial) : sfTkIUART(), _hwSerial(&hwSerial)
70+
sfTkArdUART(arduino::HardwareSerial &hwSerial) : sfTkIUART(), _hwSerial{&hwSerial}
6971
{
7072
}
7173

7274
/**
7375
* @brief Copy Constructor
7476
*/
75-
sfTkArdUART(sfTkArdUART const &rhs) : sfTkIUART(rhs._config), _hwSerial(rhs._hwSerial)
77+
sfTkArdUART(sfTkArdUART const &rhs) : sfTkIUART(rhs._config), _hwSerial{rhs._hwSerial}
7678
{
7779
}
7880

@@ -84,8 +86,11 @@ class sfTkArdUART : public sfTkIUART
8486
*/
8587
sfTkArdUART &operator=(const sfTkArdUART &rhs)
8688
{
87-
_hwSerial = rhs._hwSerial;
88-
_config = rhs._config;
89+
if(this != &rhs)
90+
{
91+
sfTkIUART::operator=(rhs);
92+
_hwSerial = rhs._hwSerial;
93+
}
8994
return *this;
9095
}
9196

@@ -214,7 +219,10 @@ class sfTkArdUART : public sfTkIUART
214219
* @param config The config struct to set
215220
* @return sfTkError_t Returns ksfTkErrOk on success, or ksfTkErrFail code
216221
*/
217-
sfTkError_t setConfig(const sfTkUARTConfig_t config) override;
222+
sfTkError_t setConfig(const uint32_t baudRate = kDefaultBaudRate,
223+
const sfTkUARTDataBits_t dataBits = kDefaultDataBits,
224+
const sfTkUARTParity_t parity = kDefaultParity,
225+
const sfTkUARTStopBits_t stopBits = kDefaultStopBits) override;
218226

219227
/**
220228
* @brief Arduino HardwareSerial functionality mappings.
@@ -316,7 +324,7 @@ class sfTkArdUARTBus : public sfTkISerialBus
316324
* @brief Constructor for the UART bus
317325
*
318326
*/
319-
sfTkArdUARTBus(void) : sfTkISerialBus(), _uartPort(nullptr)
327+
sfTkArdUARTBus(void) : sfTkISerialBus(), _uartPort{nullptr}
320328
{
321329
}
322330

@@ -325,7 +333,7 @@ class sfTkArdUARTBus : public sfTkISerialBus
325333
*
326334
* @param uartPort UART port to use
327335
*/
328-
sfTkArdUARTBus(sfTkArdUART &uartPort) : sfTkISerialBus(), _uartPort(&uartPort)
336+
sfTkArdUARTBus(sfTkArdUART &uartPort) : sfTkISerialBus(), _uartPort{std::make_unique<sfTkArdUART>(uartPort)}
329337
{
330338
}
331339

@@ -334,17 +342,16 @@ class sfTkArdUARTBus : public sfTkISerialBus
334342
*
335343
* @param hwSerial Pass in an underlying hardware serial port
336344
*/
337-
sfTkArdUARTBus(arduino::HardwareSerial &hwSerial) : sfTkISerialBus()
345+
sfTkArdUARTBus(arduino::HardwareSerial &hwSerial) : sfTkISerialBus(), _uartPort{std::make_unique<sfTkArdUART>(hwSerial)}
338346
{
339-
_uartPort = new sfTkArdUART(hwSerial);
340347
}
341348

342349
/**
343350
* @brief Copy constructer
344351
*
345352
* @param rhs Bus object to be copied
346353
*/
347-
sfTkArdUARTBus(sfTkArdUARTBus const &rhs) : sfTkISerialBus(), _uartPort(rhs._uartPort)
354+
sfTkArdUARTBus(sfTkArdUARTBus const &rhs) : sfTkISerialBus(), _uartPort{std::make_unique<sfTkArdUART>(*rhs._uartPort)}
348355
{
349356
}
350357

@@ -356,7 +363,17 @@ class sfTkArdUARTBus : public sfTkISerialBus
356363
*/
357364
sfTkArdUARTBus &operator=(const sfTkArdUARTBus &rhs)
358365
{
359-
_uartPort = rhs._uartPort;
366+
if(this != &rhs)
367+
{
368+
if(rhs._uartPort)
369+
{
370+
_uartPort = std::make_unique<sfTkArdUART>(*rhs._uartPort);
371+
}
372+
else
373+
{
374+
_uartPort.reset();
375+
}
376+
}
360377
return *this;
361378
}
362379

@@ -369,7 +386,7 @@ class sfTkArdUARTBus : public sfTkISerialBus
369386
sfTkError_t init(void)
370387
{
371388
if(!_uartPort)
372-
_uartPort = new sfTkArdUART();
389+
_uartPort = std::make_unique<sfTkArdUART>();
373390
return _uartPort->init();
374391
}
375392

@@ -381,7 +398,7 @@ class sfTkArdUARTBus : public sfTkISerialBus
381398
sfTkError_t init(uint32_t baudRate, bool bInit = false)
382399
{
383400
if(!_uartPort)
384-
_uartPort = new sfTkArdUART();
401+
_uartPort = std::make_unique<sfTkArdUART>();
385402
return _uartPort->init(baudRate, bInit);
386403
}
387404

@@ -393,7 +410,7 @@ class sfTkArdUARTBus : public sfTkISerialBus
393410
sfTkError_t init(sfTkUARTConfig_t config, bool bInit = false)
394411
{
395412
if(!_uartPort)
396-
_uartPort = new sfTkArdUART();
413+
_uartPort = std::make_unique<sfTkArdUART>();
397414
return _uartPort->init(config, bInit);
398415
}
399416

@@ -407,7 +424,7 @@ class sfTkArdUARTBus : public sfTkISerialBus
407424
*/
408425
sfTkError_t init(sfTkArdUART &uartPort, uint32_t baudRate, bool bInit = false)
409426
{
410-
_uartPort = &uartPort;
427+
_uartPort = std::make_unique<sfTkArdUART>(uartPort);
411428
return _uartPort->init(baudRate, bInit);
412429
}
413430

@@ -421,7 +438,7 @@ class sfTkArdUARTBus : public sfTkISerialBus
421438
*/
422439
sfTkError_t init(sfTkArdUART &uartPort, sfTkUARTConfig_t config, bool bInit = false)
423440
{
424-
_uartPort = &uartPort;
441+
_uartPort = std::make_unique<sfTkArdUART>(uartPort);
425442
return _uartPort->init(config, bInit);
426443
}
427444

@@ -434,7 +451,7 @@ class sfTkArdUARTBus : public sfTkISerialBus
434451
*/
435452
sfTkError_t init(sfTkArdUART &uartPort, bool bInit = false)
436453
{
437-
_uartPort = &uartPort;
454+
_uartPort = std::make_unique<sfTkArdUART>(uartPort);
438455
return _uartPort->init(sfTkIUART::kDefaultBaudRate, bInit);
439456
}
440457

@@ -448,7 +465,7 @@ class sfTkArdUARTBus : public sfTkISerialBus
448465
*/
449466
sfTkError_t init(arduino::HardwareSerial &hwSerial, uint32_t baudRate, bool bInit = false)
450467
{
451-
_uartPort = new sfTkArdUART(hwSerial);
468+
_uartPort = std::make_unique<sfTkArdUART>(hwSerial);
452469
return _uartPort->init(baudRate, bInit);
453470
}
454471

@@ -462,7 +479,7 @@ class sfTkArdUARTBus : public sfTkISerialBus
462479
*/
463480
sfTkError_t init(arduino::HardwareSerial &hwSerial, sfTkUARTConfig_t config, bool bInit = false)
464481
{
465-
_uartPort = new sfTkArdUART(hwSerial);
482+
_uartPort = std::make_unique<sfTkArdUART>(hwSerial);
466483
return _uartPort->init(config, bInit);
467484
}
468485

@@ -475,7 +492,7 @@ class sfTkArdUARTBus : public sfTkISerialBus
475492
*/
476493
sfTkError_t init(arduino::HardwareSerial &hwSerial, bool bInit = false)
477494
{
478-
_uartPort = new sfTkArdUART(hwSerial);
495+
_uartPort = std::make_unique<sfTkArdUART>(hwSerial);
479496
return _uartPort->init(sfTkIUART::kDefaultBaudRate, bInit);
480497
}
481498

@@ -513,5 +530,5 @@ class sfTkArdUARTBus : public sfTkISerialBus
513530

514531
protected:
515532
/** The actual UART port */
516-
sfTkArdUART *_uartPort;
533+
std::unique_ptr<sfTkArdUART> _uartPort;
517534
};

0 commit comments

Comments
 (0)