Skip to content

Commit e9296b5

Browse files
authored
feat(base_peripheral): Update mutex to be mutable and make all read methods const. (#538)
* feat(base_peripheral): Update mutex to be `mutable` and make all `read` methods `const`. * remove extraneous ec check * fix accidental change * remove unnecessary locking
1 parent 3fb544c commit e9296b5

File tree

1 file changed

+96
-63
lines changed

1 file changed

+96
-63
lines changed

components/base_peripheral/include/base_peripheral.hpp

Lines changed: 96 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ class BasePeripheral : public BaseComponent {
100100
/// \note If the probe function is not set, this function will return false
101101
/// and set the error code to operation_not_supported
102102
/// \note This function is only available if UseAddress is true
103-
bool probe(std::error_code &ec) requires(UseAddress) {
103+
bool probe(std::error_code &ec) const requires(UseAddress) {
104104
std::lock_guard<std::recursive_mutex> lock(base_mutex_);
105105
if (base_config_.probe) {
106106
return base_config_.probe(base_config_.address);
@@ -213,6 +213,14 @@ class BasePeripheral : public BaseComponent {
213213
base_config_ = std::move(config);
214214
}
215215

216+
/// Get the configuration for the peripheral
217+
/// \return The configuration for the peripheral
218+
const Config &config() const { return base_config_; }
219+
220+
/// Get the address of the peripheral
221+
/// \return The address of the peripheral
222+
uint8_t address() const { return base_config_.address; }
223+
216224
protected:
217225
/// Constructor
218226
/// \param config The configuration for the peripheral
@@ -223,14 +231,6 @@ class BasePeripheral : public BaseComponent {
223231
: BaseComponent(name, verbosity)
224232
, base_config_(config) {}
225233

226-
/// Get the configuration for the peripheral
227-
/// \return The configuration for the peripheral
228-
const Config &config() const { return base_config_; }
229-
230-
/// Get the address of the peripheral
231-
/// \return The address of the peripheral
232-
uint8_t address() const { return base_config_.address; }
233-
234234
/// Write data to the peripheral
235235
/// \param data The data to write
236236
/// \param length The length of the data to write
@@ -269,7 +269,7 @@ class BasePeripheral : public BaseComponent {
269269
/// \param data The buffer to read into
270270
/// \param length The length of the buffer
271271
/// \param ec The error code to set if there is an error
272-
void read(uint8_t *data, size_t length, std::error_code &ec) requires(UseAddress) {
272+
void read(uint8_t *data, size_t length, std::error_code &ec) const requires(UseAddress) {
273273
std::lock_guard<std::recursive_mutex> lock(base_mutex_);
274274
if (base_config_.read) {
275275
if (!base_config_.read(base_config_.address, data, length)) {
@@ -286,7 +286,7 @@ class BasePeripheral : public BaseComponent {
286286
/// \param data The buffer to read into
287287
/// \param length The length of the buffer
288288
/// \param ec The error code to set if there is an error
289-
void read(uint8_t *data, size_t length, std::error_code &ec) requires(!UseAddress) {
289+
void read(uint8_t *data, size_t length, std::error_code &ec) const requires(!UseAddress) {
290290
std::lock_guard<std::recursive_mutex> lock(base_mutex_);
291291
if (base_config_.read) {
292292
if (!base_config_.read(data, length)) {
@@ -305,7 +305,7 @@ class BasePeripheral : public BaseComponent {
305305
/// \param length The length of the buffer
306306
/// \param ec The error code to set if there is an error
307307
void read_register(RegisterAddressType reg_addr, uint8_t *data, size_t length,
308-
std::error_code &ec) requires(UseAddress) {
308+
std::error_code &ec) const requires(UseAddress) {
309309
std::lock_guard<std::recursive_mutex> lock(base_mutex_);
310310
if (base_config_.read_register) {
311311
if (!base_config_.read_register(base_config_.address, reg_addr, data, length)) {
@@ -314,7 +314,39 @@ class BasePeripheral : public BaseComponent {
314314
ec.clear();
315315
}
316316
} else {
317-
ec = std::make_error_code(std::errc::operation_not_supported);
317+
// use the size of the register address to determine how many bytes the
318+
// register address is
319+
static constexpr size_t reg_addr_size = sizeof(RegisterAddressType);
320+
uint8_t buffer[reg_addr_size];
321+
put_register_bytes(reg_addr, buffer);
322+
323+
// now we need to determine if we can write_then_read, or if we need to
324+
// write then read separately
325+
if (base_config_.write_then_read) {
326+
// we can use write_then_read
327+
if (!base_config_.write_then_read(base_config_.address, buffer, reg_addr_size, data,
328+
length)) {
329+
ec = std::make_error_code(std::errc::io_error);
330+
} else {
331+
ec.clear();
332+
}
333+
} else if (base_config_.write && base_config_.read) {
334+
if (!base_config_.write(base_config_.address, buffer, reg_addr_size)) {
335+
ec = std::make_error_code(std::errc::io_error);
336+
return;
337+
}
338+
if (base_config_.separate_write_then_read_delay.count() > 0) {
339+
std::this_thread::sleep_for(base_config_.separate_write_then_read_delay);
340+
}
341+
if (!base_config_.read(base_config_.address, data, length)) {
342+
ec = std::make_error_code(std::errc::io_error);
343+
return;
344+
}
345+
} else {
346+
// we can't do anything
347+
ec = std::make_error_code(std::errc::operation_not_supported);
348+
return;
349+
}
318350
}
319351
}
320352

@@ -324,7 +356,7 @@ class BasePeripheral : public BaseComponent {
324356
/// \param length The length of the buffer
325357
/// \param ec The error code to set if there is an error
326358
void read_register(RegisterAddressType reg_addr, uint8_t *data, size_t length,
327-
std::error_code &ec) requires(!UseAddress) {
359+
std::error_code &ec) const requires(!UseAddress) {
328360
std::lock_guard<std::recursive_mutex> lock(base_mutex_);
329361
if (base_config_.read_register) {
330362
if (!base_config_.read_register(reg_addr, data, length)) {
@@ -333,7 +365,38 @@ class BasePeripheral : public BaseComponent {
333365
ec.clear();
334366
}
335367
} else {
336-
ec = std::make_error_code(std::errc::operation_not_supported);
368+
// use the size of the register address to determine how many bytes the
369+
// register address is
370+
static constexpr size_t reg_addr_size = sizeof(RegisterAddressType);
371+
uint8_t buffer[reg_addr_size];
372+
put_register_bytes(reg_addr, buffer);
373+
374+
// now we need to determine if we can write_then_read, or if we need to
375+
// write then read separately
376+
if (base_config_.write_then_read) {
377+
// we can use write_then_read
378+
if (!base_config_.write_then_read(buffer, reg_addr_size, data, length)) {
379+
ec = std::make_error_code(std::errc::io_error);
380+
} else {
381+
ec.clear();
382+
}
383+
} else if (base_config_.write && base_config_.read) {
384+
if (!base_config_.write(buffer, reg_addr_size)) {
385+
ec = std::make_error_code(std::errc::io_error);
386+
return;
387+
}
388+
if (base_config_.separate_write_then_read_delay.count() > 0) {
389+
std::this_thread::sleep_for(base_config_.separate_write_then_read_delay);
390+
}
391+
if (!base_config_.read(data, length)) {
392+
ec = std::make_error_code(std::errc::io_error);
393+
return;
394+
}
395+
} else {
396+
// we can't do anything
397+
ec = std::make_error_code(std::errc::operation_not_supported);
398+
return;
399+
}
337400
}
338401
}
339402

@@ -445,7 +508,7 @@ class BasePeripheral : public BaseComponent {
445508
/// \param data The buffer to read into
446509
/// \param length The length of the buffer
447510
/// \param ec The error code to set if there is an error
448-
void read_many(uint8_t *data, size_t length, std::error_code &ec) {
511+
void read_many(uint8_t *data, size_t length, std::error_code &ec) const {
449512
logger_.debug("read {} bytes", length);
450513
read(data, length, ec);
451514
}
@@ -456,15 +519,15 @@ class BasePeripheral : public BaseComponent {
456519
/// \note You should ensure that the buffer is the correct size before calling
457520
/// this function since the buffer size is what is used to know how many
458521
/// bytes to read
459-
void read_many(std::vector<uint8_t> &data, std::error_code &ec) {
522+
void read_many(std::vector<uint8_t> &data, std::error_code &ec) const {
460523
logger_.debug("read {} bytes", data.size());
461524
read(data.data(), data.size(), ec);
462525
}
463526

464527
/// Read a uint8_t from the peripheral
465528
/// \param ec The error code to set if there is an error
466529
/// \return The data read from the peripheral
467-
uint8_t read_u8(std::error_code &ec) {
530+
uint8_t read_u8(std::error_code &ec) const {
468531
logger_.debug("read u8");
469532
uint8_t data = 0;
470533
read(&data, 1, ec);
@@ -477,7 +540,7 @@ class BasePeripheral : public BaseComponent {
477540
/// Read a uint16_t from the peripheral
478541
/// \param ec The error code to set if there is an error
479542
/// \return The data read from the peripheral
480-
uint16_t read_u16(std::error_code &ec) {
543+
uint16_t read_u16(std::error_code &ec) const {
481544
logger_.debug("read u16");
482545
uint16_t data = 0;
483546
uint8_t buffer[2];
@@ -559,22 +622,12 @@ class BasePeripheral : public BaseComponent {
559622
/// \param register_address The address of the register to read from
560623
/// \param ec The error code to set if there is an error
561624
/// \return The data read from the peripheral
562-
uint8_t read_u8_from_register(RegisterAddressType register_address, std::error_code &ec) {
625+
uint8_t read_u8_from_register(RegisterAddressType register_address, std::error_code &ec) const {
563626
logger_.debug("read u8 from register 0x{:x}", register_address);
564627
uint8_t data = 0;
565-
std::lock_guard<std::recursive_mutex> lock(base_mutex_);
566-
if (base_config_.read_register) {
567-
read_register(register_address, &data, 1, ec);
568-
} else {
569-
// use the size of the register address to determine how many bytes the
570-
// register address is
571-
static constexpr size_t reg_addr_size = sizeof(RegisterAddressType);
572-
uint8_t buffer[reg_addr_size];
573-
put_register_bytes(register_address, buffer);
574-
write_then_read(buffer, reg_addr_size, &data, 1, ec);
575-
if (ec) {
576-
return 0;
577-
}
628+
read_register(register_address, &data, 1, ec);
629+
if (ec) {
630+
return 0;
578631
}
579632
return data;
580633
}
@@ -583,22 +636,12 @@ class BasePeripheral : public BaseComponent {
583636
/// \param register_address The address of the register to read from
584637
/// \param ec The error code to set if there is an error
585638
/// \return The data read from the peripheral
586-
uint16_t read_u16_from_register(RegisterAddressType register_address, std::error_code &ec) {
639+
uint16_t read_u16_from_register(RegisterAddressType register_address, std::error_code &ec) const {
587640
logger_.debug("read u16 from register 0x{:x}", register_address);
588641
uint8_t data[2];
589-
std::lock_guard<std::recursive_mutex> lock(base_mutex_);
590-
if (base_config_.read_register) {
591-
read_register(register_address, data, 2, ec);
592-
} else {
593-
// use the size of the register address to determine how many bytes the
594-
// register address is
595-
static constexpr size_t reg_addr_size = sizeof(RegisterAddressType);
596-
uint8_t buffer[reg_addr_size];
597-
put_register_bytes(register_address, buffer);
598-
write_then_read(buffer, reg_addr_size, (uint8_t *)&data, 2, ec);
599-
if (ec) {
600-
return 0;
601-
}
642+
read_register(register_address, data, 2, ec);
643+
if (ec) {
644+
return 0;
602645
}
603646
if constexpr (BigEndianData) {
604647
return (data[0] << 8) | data[1];
@@ -613,28 +656,18 @@ class BasePeripheral : public BaseComponent {
613656
/// \param length The length of the buffer
614657
/// \param ec The error code to set if there is an error
615658
void read_many_from_register(RegisterAddressType register_address, uint8_t *data, size_t length,
616-
std::error_code &ec) {
659+
std::error_code &ec) const {
617660
logger_.debug("read_many_from_register {} bytes from register 0x{:x}", length,
618661
register_address);
619-
std::lock_guard<std::recursive_mutex> lock(base_mutex_);
620-
if (base_config_.read_register) {
621-
read_register(register_address, data, length, ec);
622-
} else {
623-
// use the size of the register address to determine how many bytes the
624-
// register address is
625-
static constexpr size_t reg_addr_size = sizeof(RegisterAddressType);
626-
uint8_t buffer[reg_addr_size];
627-
put_register_bytes(register_address, buffer);
628-
write_then_read(buffer, reg_addr_size, data, length, ec);
629-
}
662+
read_register(register_address, data, length, ec);
630663
}
631664

632665
/// Read many bytes from a register on the peripheral
633666
/// \param register_address The address of the register to read from
634667
/// \param data The buffer to read into
635668
/// \param ec The error code to set if there is an error
636669
void read_many_from_register(RegisterAddressType register_address, std::vector<uint8_t> &data,
637-
std::error_code &ec) {
670+
std::error_code &ec) const {
638671
logger_.debug("read_many_from_register {} bytes from register 0x{:x}", data.size(),
639672
register_address);
640673
read_many_from_register(register_address, data.data(), data.size(), ec);
@@ -771,7 +804,7 @@ class BasePeripheral : public BaseComponent {
771804
}
772805

773806
// Set the bytes of the register address in the buffer
774-
void put_register_bytes(RegisterAddressType register_address, uint8_t *data) {
807+
void put_register_bytes(RegisterAddressType register_address, uint8_t *data) const {
775808
if constexpr (std::is_same_v<RegisterAddressType, uint8_t>) {
776809
data[0] = register_address;
777810
} else {
@@ -794,7 +827,7 @@ class BasePeripheral : public BaseComponent {
794827
}
795828
}
796829

797-
Config base_config_; ///< The configuration for the peripheral
798-
std::recursive_mutex base_mutex_; ///< The mutex to protect access to the peripheral
830+
Config base_config_; ///< The configuration for the peripheral
831+
mutable std::recursive_mutex base_mutex_; ///< The mutex to protect access to the peripheral
799832
};
800833
} // namespace espp

0 commit comments

Comments
 (0)