2
\$\begingroup\$

On this platform I have been developing a software driver for two digital inputs expanders communicating over the SPI. My C++ code is based on the SPI driver which accompanies the SDK offered by the MCU manufacturer. From the timing perspective the driver has been conceived as non-blocking with periodic updates exploiting callback function. From the architectural point of view the driver has been designed in following manner:

  • the top level layer of the driver consists of the DigitalInputsDriver class which provides the interface for the application layer of the software
  • the digital inputs expander is modeled by the MAX22190 class
  • the expander is basically a set of registers. This is reflected in design by the fact that the MAX22190 class contains an array of registers (instances of the Register class). Those instances are "mirrors" of the real hw registers in the MAX22190 chips. Content of those mirrors is held in consistent state with their hw counterparts based on algorithms for configuration and refreshing encapsulated in the classes Configurator and Refresher. Based on stimulus comming from the Configurator and Refresher the Register objects communicate with the MAX22190 chips over SPI via messages which are instances of WriteRegRequestMsg, ReadRegRequestMsg, WriteRegResponseMsg and ReadRegResponseMsg classes which have same interface defined by the Message abstract class.
  • each MAX22190 can be richly configured, so DigitalInputsDriverCfg class has been defined which manages the configuration.

DigitalInputsDriver

#include "DigitalInputsDriverCfg.h"
#include "MAX22190.h"
#include "Transceiver.h"
#include <cstdint>

class DigitalInputsDriver
{
public:
  enum class Input{
    kDi_00,
    kDi_01,
    kDi_02,
    kDi_03,
    kDi_04,
    kDi_05,
    kDi_06,
    kDi_07,
    kDi_08,
    kDi_09,
    kDi_10,
    kDi_11,
    kDi_12,
    kDi_13,
    kDi_14,
    kDi_15,
    kNoDigitalInputs
  };

  enum class State{kLow, kHigh};

  enum class Fault1{
    kWireBreakDevice0,
    k24VMDevice0,
    k24VLDevice0,
    kOverTemperature1Device0,
    kOverTemperature2Device0,
    kFault2Device0,
    kPorDevice0,
    kCrcDevice0,
    kWireBreakDevice1,
    k24VMDevice1,
    k24VLDevice1,
    kOverTemperature1Device1,
    kOverTemperature2Device1,
    kFault2Device1,
    kPorDevice1,
    kCrcDevice1
  };

  enum class Fault2{
    kREFWBShortDevice0,
    kREFWBOpenDevice0,
    kREFDIShortDevice0,
    kREFDIOpenDevice0,
    kOverTempShutdownDevice0,
    kFault8ClkDevice0,
    kREFWBShortDevice1 = 8,
    kREFWBOpenDevice1  = 9,
    kREFDIShortDevice1 = 10,
    kREFDIOpenDevice1  = 11,
    kOverTempShutdownDevice1 = 12,
    kFault8ClkDevice1 = 13,
  };

  DigitalInputsDriver(DigitalInputsDriverCfg *_dig_in_cfg, uint16_t _spi_device_id);
  void update(void);
  void initialize(void);
  bool isReady(void);
  State getInputState(Input input);
  bool isFault1Active(Fault1 fault);
  bool isFault2Active(Fault2 fault);
  void handleFpgaProtection(void);
  void handleSpiEndOfTransactionInterrupt(void);

private:
Transceiver transceiver;
MAX22190 device0;
MAX22190 device1;
MAX22190 *devices[static_cast<uint8_t>(DigitalInputsDriverCfg::Device::kNoMAX22190Devices)];
};

#include "DigitalInputsDriver.h"

DigitalInputsDriver::DigitalInputsDriver(DigitalInputsDriverCfg *_dig_in_cfg,
  uint16_t _spi_device_id) :
  transceiver(_spi_device_id),
  device0(DigitalInputsDriverCfg::Device::kMAX22190Device_0, _dig_in_cfg, &transceiver),
  device1(DigitalInputsDriverCfg::Device::kMAX22190Device_1, _dig_in_cfg, &transceiver)
 {         
  devices[static_cast<uint8_t>(DigitalInputsDriverCfg::Device::kMAX22190Device_0)] =
      &device0;
  devices[static_cast<uint8_t>(DigitalInputsDriverCfg::Device::kMAX22190Device_1)] =
      &device1;
}

void DigitalInputsDriver::initialize(void)
{
  transceiver.initialize();
  for(MAX22190 *device : devices){
    device->initialize();
  }
}

void DigitalInputsDriver::update(void)
{
  for(MAX22190 *device : devices){
    device->update();
  }
}

bool DigitalInputsDriver::isReady(void)
{
  bool retval = true;
  for(MAX22190 *device : devices){
    if(device->isReady() == false){
      retval = false;
      break;
    }
  }
  return retval;
}

DigitalInputsDriver::State DigitalInputsDriver::getInputState(Input input)
{
  return static_cast<DigitalInputsDriver::State>(devices[(static_cast<uint8_t>(input) >> 3)]->isInputActive(static_cast<DigitalInputsDriverCfg::Input>(static_cast<uint8_t>(input) - ((static_cast<uint8_t>(input) >> 3) << 3))));
}

bool DigitalInputsDriver::isFault1Active(Fault1 fault)
{
  return devices[(static_cast<uint8_t>(fault) >> 3)]->isFault1Active(static_cast<MAX22190::Fault1>(static_cast<uint8_t>(fault) - ((static_cast<uint8_t>(fault) >> 3) << 3)));
}

bool DigitalInputsDriver::isFault2Active(Fault2 fault)
{
  return devices[(static_cast<uint8_t>(fault) >> 3)]->isFault2Active(static_cast<MAX22190::Fault2>(static_cast<uint8_t>(fault) - ((static_cast<uint8_t>(fault) >> 3) << 3)));
}
    
void DigitalInputsDriver::handleFpgaProtection(void)
{
  for(MAX22190 *device : devices){
    device->handleFaultPinActivation();
  }
}

void DigitalInputsDriver::handleSpiEndOfTransactionInterrupt(void)
{
   for(MAX22190 *device : devices){
     device->notifyEndOfTransaction();
   }
}

MAX22190

#include "Register.h"
#include "DigitalInputsDriverCfg.h"
#include "Configurator.h"
#include "TransactionEndListener.h"
#include "Transceiver.h"
#include "Refresher.h"

class MAX22190 : public TransactionEndListener
{
  friend class Configurator;
  friend class Refresher;

public:
  enum class Fault1{
    kWireBreak,
    k24VM,
    k24VL,
    kOverTemperature1,
    kOverTemperature2,
    kFault2,
    kPor,
    kCrc
  };

  enum class Fault2{
    kREFWBShort,
    kREFWBOpen,
    kREFDIShort,
    kREFDIOpen,
    kOvertempShd,
    kFault8Clk
  };

  MAX22190(DigitalInputsDriverCfg::Device _device, DigitalInputsDriverCfg *_configuration, 
        Transceiver *_transceiver);
  void initialize(void);
  void update(void);
  bool isReady(void);
  bool isInputActive(DigitalInputsDriverCfg::Input input);
  bool isFault1Active(Fault1 fault);
  bool isFault2Active(Fault2 fault);
  void handleFaultPinActivation(void);
  void activateLatch(void);
  uint8_t getDeviceId(void);    
  void notifyEndOfTransaction(void);

private:
  enum class ConfigurationState{
    kConfigInEnReg,
    kConfigFlt1Reg,
    kConfigFlt2Reg,
    kConfigFlt3Reg,
    kConfigFlt4Reg,
    kConfigFlt5Reg,
    kConfigFlt6Reg,
    kConfigFlt7Reg,
    kConfigFlt8Reg,
    kConfigFault2EnReg,
    kConfigFault1EnReg,
    kConfigCfgReg,
    kConfigGpoReg,
    kConfigurationEnd
  };
  
  enum class RefreshState{
      kRefreshFault1Reg,
      kRefreshFault2Reg,
      kRefreshDiReg,
      kRefreshWbReg
  };
  
  static constexpr uint8_t wb_reg_addr = 0x00;
  static constexpr uint8_t di_reg_addr = 0x02;
  static constexpr uint8_t fault1_reg_addr = 0x04;
  static constexpr uint8_t flt1_reg_addr = 0x06;
  static constexpr uint8_t flt2_reg_addr = 0x08;
  static constexpr uint8_t flt3_reg_addr = 0x0A;
  static constexpr uint8_t flt4_reg_addr = 0x0C;
  static constexpr uint8_t flt5_reg_addr = 0x0E;
  static constexpr uint8_t flt6_reg_addr = 0x10;
  static constexpr uint8_t flt7_reg_addr = 0x12;
  static constexpr uint8_t flt8_reg_addr = 0x14;
  static constexpr uint8_t cfg_reg_addr = 0x18;
  static constexpr uint8_t inen_reg_addr = 0x1A;
  static constexpr uint8_t fault2_reg_addr = 0x1C;
  static constexpr uint8_t fault2en_reg_addr = 0x1E;
  static constexpr uint8_t gpo_reg_addr = 0x22;
  static constexpr uint8_t fault1en_reg_addr = 0x24;
  static constexpr uint8_t nop_reg_addr = 0x26;
  static constexpr uint8_t no_regs = 18;
  Register wb_reg;
  Register di_reg;
  Register fault1_reg;
  Register flt1_reg;
  Register flt2_reg;
  Register flt3_reg;
  Register flt4_reg;
  Register flt5_reg;
  Register flt6_reg;
  Register flt7_reg;
  Register flt8_reg;
  Register cfg_reg;
  Register inen_reg;
  Register fault2_reg;
  Register fault2en_reg;
  Register gpo_reg;
  Register fault1en_reg;
  Register nop_reg;
  Register* register_map[no_regs];
  Configurator configurator;
  Refresher refresher;
  DigitalInputsDriverCfg::Device device;
  DigitalInputsDriverCfg *configuration;
  Transceiver *transceiver;
  bool device_ready;
  bool device_configured;
  bool read_faults;
  ConfigurationState state_config;
  RefreshState state_refresh;
  
  bool configure(void);
  void configureInEnReg(void);
  void configureFlt1Reg(void);
  void configureFlt2Reg(void);
  void configureFlt3Reg(void);
  void configureFlt4Reg(void);
  void configureFlt5Reg(void);
  void configureFlt6Reg(void);
  void configureFlt7Reg(void);
  void configureFlt8Reg(void);
  void configureFault2EnReg(void);
  void configureFault1EnReg(void);
  void configureCfgReg(void);
  void configureGpoReg(void);     
  void refresh(void);
  void refreshDiReg(void);
  void refreshWbReg(void);
  void refreshFault1Reg(void);
  void refreshFault2Reg(void);
};

#include "MAX22190.h"
    
MAX22190::MAX22190(DigitalInputsDriverCfg::Device _device, DigitalInputsDriverCfg *_configuration,
                   Transceiver *_transceiver) :
  wb_reg(_device, DigitalInputsDriverCfg::MAX22190RegisterType::kWb, wb_reg_addr, _transceiver),
  di_reg(_device, DigitalInputsDriverCfg::MAX22190RegisterType::kDi, di_reg_addr, _transceiver),
  fault1_reg(_device, DigitalInputsDriverCfg::MAX22190RegisterType::kFault1,
             fault1_reg_addr, _transceiver),
  flt1_reg(_device, DigitalInputsDriverCfg::MAX22190RegisterType::kFlt1,
           flt1_reg_addr, _transceiver),
  flt2_reg(_device, DigitalInputsDriverCfg::MAX22190RegisterType::kFlt2,
           flt2_reg_addr, _transceiver),
  flt3_reg(_device, DigitalInputsDriverCfg::MAX22190RegisterType::kFlt3,
           flt3_reg_addr, _transceiver),
  flt4_reg(_device, DigitalInputsDriverCfg::MAX22190RegisterType::kFlt4,
           flt4_reg_addr, _transceiver),
  flt5_reg(_device, DigitalInputsDriverCfg::MAX22190RegisterType::kFlt5,
           flt5_reg_addr, _transceiver),
  flt6_reg(_device, DigitalInputsDriverCfg::MAX22190RegisterType::kFlt6,
           flt6_reg_addr, _transceiver),
  flt7_reg(_device, DigitalInputsDriverCfg::MAX22190RegisterType::kFlt7,
           flt7_reg_addr, _transceiver),
  flt8_reg(_device, DigitalInputsDriverCfg::MAX22190RegisterType::kFlt8,
           flt8_reg_addr, _transceiver),
  cfg_reg(_device, DigitalInputsDriverCfg::MAX22190RegisterType::kCfg, cfg_reg_addr, _transceiver),
  inen_reg(_device, DigitalInputsDriverCfg::MAX22190RegisterType::kInEn,
           inen_reg_addr, _transceiver),
  fault2_reg(_device, DigitalInputsDriverCfg::MAX22190RegisterType::kFault2,
             fault2_reg_addr, _transceiver),
  fault2en_reg(_device, DigitalInputsDriverCfg::MAX22190RegisterType::kFault2En,
               fault2en_reg_addr, _transceiver),
  gpo_reg(_device, DigitalInputsDriverCfg::MAX22190RegisterType::kGpo, gpo_reg_addr, _transceiver),
  fault1en_reg(_device, DigitalInputsDriverCfg::MAX22190RegisterType::kFault1En,
               fault1en_reg_addr, _transceiver),
  nop_reg(_device, DigitalInputsDriverCfg::MAX22190RegisterType::kNop, nop_reg_addr, _transceiver),
  configurator(_configuration),
  refresher()
{        
  register_map[0] = &wb_reg;
  register_map[1] = &di_reg;
  register_map[2] = &fault1_reg;
  register_map[3] = &flt1_reg;
  register_map[4] = &flt2_reg;
  register_map[5] = &flt3_reg;
  register_map[6] = &flt4_reg;
  register_map[7] = &flt5_reg;
  register_map[8] = &flt6_reg;
  register_map[9] = &flt7_reg;
  register_map[10]= &flt8_reg;
  register_map[11]= &cfg_reg;
  register_map[12]= &inen_reg;
  register_map[13]= &fault2_reg;
  register_map[14]= &fault2en_reg;
  register_map[15]= &gpo_reg;
  register_map[16]= &fault1en_reg;
  register_map[17]= &nop_reg; 
  device = _device;
  device_ready = false;
  device_configured = false;
  read_faults = false;
  configuration = _configuration;
  state_config = ConfigurationState::kConfigInEnReg;
  state_refresh = RefreshState::kRefreshDiReg;
  transceiver = _transceiver;
}

void MAX22190::initialize(void){
  transceiver->registerListener(this);
  device_ready = true;
}

void MAX22190::update(void)
{
  if(device_ready){  
    if(!device_configured){
      device_configured = configure();
    }else{
    refresh();
  }
}
}

bool MAX22190::isReady(void)
{
  return (device_ready && device_configured);
}

bool MAX22190::isInputActive(DigitalInputsDriverCfg::Input input)
{
  uint8_t reg_data = di_reg.getData();
  return ((reg_data & (1 << static_cast<uint8_t>(input))) != 0);
}

bool MAX22190::isFault1Active(Fault1 fault)
{
  uint8_t reg_data = fault1_reg.getData();
  return ((reg_data & (1 << static_cast<uint8_t>(fault))) != 0);
}

bool MAX22190::isFault2Active(Fault2 fault)
{
  uint8_t reg_data = fault2_reg.getData();
  return ((reg_data & (1 << static_cast<uint8_t>(fault))) != 0);
}

void MAX22190::handleFaultPinActivation(void)
{
    read_faults = true;
}

void MAX22190::activateLatch(void){}

void MAX22190::notifyEndOfTransaction(void){
  for(Register* reg : register_map){
    if(reg->isPending()){
      reg->handleEndOfTransaction();
      break;
    }
  }
}

 uint8_t MAX22190::getDeviceId(void){
     return static_cast<uint8_t>(device);
 }
    
bool MAX22190::configure(void)
{
  bool configuration_finished = false;

  switch(state_config){
    case ConfigurationState::kConfigInEnReg:
      configureInEnReg();
      break;
    case ConfigurationState::kConfigFlt1Reg:
      configureFlt1Reg();
      break;
    case ConfigurationState::kConfigFlt2Reg:
      configureFlt2Reg();
      break;
    case ConfigurationState::kConfigFlt3Reg:
      configureFlt3Reg();
      break;
    case ConfigurationState::kConfigFlt4Reg:
      configureFlt4Reg();
      break;
    case ConfigurationState::kConfigFlt5Reg:
      configureFlt5Reg();
      break;
    case ConfigurationState::kConfigFlt6Reg:
      configureFlt6Reg();
      break;
    case ConfigurationState::kConfigFlt7Reg:
      configureFlt7Reg();
      break;
    case ConfigurationState::kConfigFlt8Reg:
      configureFlt8Reg();
      break;
    case ConfigurationState::kConfigFault2EnReg:
      configureFault2EnReg();
      break;
    case ConfigurationState::kConfigFault1EnReg:
      configureFault1EnReg();
      break;
    case ConfigurationState::kConfigCfgReg:
      configureCfgReg();
      break;
    case ConfigurationState::kConfigGpoReg:
      configureGpoReg();
      break;
    case ConfigurationState::kConfigurationEnd:
      configuration_finished = true;
      break;
  }
  return configuration_finished;
}

void MAX22190::refresh(void)
{
    switch(state_refresh){
        case RefreshState::kRefreshDiReg:
            refreshDiReg();
            break;
        case RefreshState::kRefreshWbReg:
            refreshWbReg();
            break;
        case RefreshState::kRefreshFault1Reg:
            refreshFault1Reg();
            break;
        case RefreshState::kRefreshFault2Reg:
            refreshFault2Reg();
            break;
    }
}

void MAX22190::configureInEnReg(void)
{
  if(configurator.configure(inen_reg)){
    state_config = ConfigurationState::kConfigFlt1Reg;
  }
}

void MAX22190::configureFlt1Reg(void)
{
  if(configurator.configure(flt1_reg)){
    state_config = ConfigurationState::kConfigFlt2Reg;
  }
}

void MAX22190::configureFlt2Reg(void)
{
  if(configurator.configure(flt2_reg)){
    state_config = ConfigurationState::kConfigFlt3Reg;
  }
}

void MAX22190::configureFlt3Reg(void)
{
  if(configurator.configure(flt3_reg)){
    state_config = ConfigurationState::kConfigFlt4Reg;
  }
}

void MAX22190::configureFlt4Reg(void)
{
  if(configurator.configure(flt4_reg)){
    state_config = ConfigurationState::kConfigFlt5Reg;
  }
}

void MAX22190::configureFlt5Reg(void)
{
  if(configurator.configure(flt5_reg)){
    state_config = ConfigurationState::kConfigFlt6Reg;
  }
}

void MAX22190::configureFlt6Reg(void)
{
  if(configurator.configure(flt6_reg)){
    state_config = ConfigurationState::kConfigFlt7Reg;
  }
}

void MAX22190::configureFlt7Reg(void)
{
  if(configurator.configure(flt7_reg)){
    state_config = ConfigurationState::kConfigFlt8Reg;
  }
}

void MAX22190::configureFlt8Reg(void)
{
  if(configurator.configure(flt8_reg)){
    state_config = ConfigurationState::kConfigFault2EnReg;
  }
}

void MAX22190::configureFault2EnReg(void)
{
  if(configurator.configure(fault2en_reg)){
    state_config = ConfigurationState::kConfigFault1EnReg;
  }
}

void MAX22190::configureFault1EnReg(void)
{
  if(configurator.configure(fault1en_reg)){
    state_config = ConfigurationState::kConfigCfgReg;
  }
}

void MAX22190::configureCfgReg(void)
{
  if(configurator.configure(cfg_reg)){
    state_config = ConfigurationState::kConfigGpoReg;
  }
}

void MAX22190::configureGpoReg(void)
{
  if(configurator.configure(gpo_reg)){
    state_config = ConfigurationState::kConfigurationEnd;
  }
}

void MAX22190::refreshDiReg(void)
{
  if(refresher.refresh(di_reg)){
      state_refresh = RefreshState::kRefreshWbReg;
  }
}

void MAX22190::refreshWbReg(void)
{
  if(refresher.refresh(wb_reg)){
      if(read_faults) {
        state_refresh = RefreshState::kRefreshFault1Reg;
      }else{
        state_refresh = RefreshState::kRefreshDiReg;
      }
  }  
}

void MAX22190::refreshFault1Reg(void)
{
    if(refresher.refresh(fault1_reg)){
        state_refresh = RefreshState::kRefreshFault2Reg; 
    }
}

void MAX22190::refreshFault2Reg(void)
{
    if(refresher.refresh(fault2_reg)){
        state_refresh = RefreshState::kRefreshDiReg; 
        read_faults = false;
    }
}

Register

#include "DigitalInputsDriverCfg.h"
#include "Transceiver.h"
#include "Message.h"
#include "WriteRegRequestMsg.h"
#include "WriteRegResponseMsg.h"
#include "ReadRegRequestMsg.h"
#include "ReadRegResponseMsg.h"
#include <cstdint>

class Register
{
public:
  Register(DigitalInputsDriverCfg::Device _device,
           DigitalInputsDriverCfg::MAX22190RegisterType _type, uint8_t _address, Transceiver *_transceiver);
  bool read(void);
  bool write(uint8_t data);
  bool isConfigured(void);
  void setConfigured(void);
  bool isRefreshed(void);
  void setUnRefreshed(void);
  uint8_t getData(void);
  uint8_t getAddress(void);
  DigitalInputsDriverCfg::MAX22190RegisterType getType(void);
  DigitalInputsDriverCfg::Device getDevice(void);
  bool isPending(void);
  void handleEndOfTransaction(void);

private:
  DigitalInputsDriverCfg::Device device;
  DigitalInputsDriverCfg::MAX22190RegisterType type;
  uint8_t address;
  uint8_t data;
  bool configured;
  bool pending;
  bool refreshed;
  Message::MessageType sent_msg;
  Transceiver *transceiver;
  WriteRegRequestMsg write_reg_request;
  ReadRegRequestMsg  read_reg_request;
  WriteRegResponseMsg write_reg_response;
  ReadRegResponseMsg read_reg_response;

  void setData(uint8_t data);
  void handleWriteRegResponseMsg(void);
  void handleReadRegResponseMsg(void);  
};

#include <new>
#include "Register.h"
#include "Bits.h"

Register::Register(DigitalInputsDriverCfg::Device _device,
                   DigitalInputsDriverCfg::MAX22190RegisterType _type, uint8_t _address,
                   Transceiver *_transceiver)
{       
  type = _type;
  address = _address;
  device = _device;
  configured = false;
  refreshed = false;
  pending = false;
  transceiver = _transceiver;
}

bool Register::read(void)
{
  if (!transceiver->isBusy()){
    new (&read_reg_request)
        ReadRegRequestMsg(device, address, transceiver);
    read_reg_request.write();
    sent_msg = Message::MessageType::kReadRegRequest;
    pending = true;
    return true;
  }else{
    return false;
  }
}

bool Register::write(uint8_t data)
{
  if(!transceiver->isBusy()){
    new (&write_reg_request)
        WriteRegRequestMsg(device, address, data, transceiver);
    write_reg_request.write();
    sent_msg = Message::MessageType::kWriteRegRequest;
    pending = true;
    return true;
  }else{
    return false;
  }
}

bool Register::isConfigured(void)
{
  return configured;
}

void Register::setConfigured(void)
{
    configured = true;
}

bool Register::isRefreshed(void)
{
    return (refreshed != false);
}

void Register::setUnRefreshed(void)
{
    refreshed = false;
}

uint8_t Register::getData(void) {
    return data;
}

uint8_t Register::getAddress(void) {
    return address;
}

DigitalInputsDriverCfg::MAX22190RegisterType Register::getType(void)
{
  return type;
}

DigitalInputsDriverCfg::Device Register::getDevice(void)
{
  return device;
}

bool Register::isPending(void)
{
  return pending;
}

void Register::handleEndOfTransaction(void)
{
  switch(sent_msg){
      case Message::MessageType::kWriteRegRequest:
      handleWriteRegResponseMsg();
      break;
      case Message::MessageType::kReadRegRequest:
      handleReadRegResponseMsg();
      break;
      case Message::MessageType::kWriteRegResponse:
      break;
      case Message::MessageType::kReadRegResponse:
      break;
  }
}

void Register::setData(uint8_t read_data)
{
    data = read_data;
}

void Register::handleWriteRegResponseMsg(void)
{
  pending = false;
  new (&write_reg_response) WriteRegResponseMsg(transceiver);
  write_reg_response.read();
}

void Register::handleReadRegResponseMsg(void)
{
  uint8_t read_data;

  pending = false;
  new (&read_reg_response) ReadRegResponseMsg(transceiver);
  read_reg_response.read();
  if(read_reg_response.isValid()){
    read_data = read_reg_response.getRegisterData();
    setData(read_data);
    refreshed = true;
  }
}

Configurator

#include "DigitalInputsDriverCfg.h"
#include "Register.h"

class Configurator
{
public:
  Configurator(DigitalInputsDriverCfg *_configuration);
  bool configure(Register &reg);

private:
  enum class State {kInit, kWrite, kRead, kCheck};

  State state;
  DigitalInputsDriverCfg *configuration;
  uint8_t data;
};

#include "Configurator.h"
    
Configurator::Configurator(DigitalInputsDriverCfg *_configuration)
{        
  configuration = _configuration;
  state = State::kInit;
}

bool Configurator::configure(Register &reg)
{
  bool reg_configured = false;
  
  switch(state){
    case State::kInit:
      data =
          configuration->getRegisterConfigData(reg.getDevice(), reg.getType());
      state = State::kWrite;
      break;

    case State::kWrite:
      if(reg.write(data)){
        state = State::kRead;
      }
      break;

    case State::kRead:
      if(reg.read()){
       state = State::kCheck; 
      }
      break;
      
    case State::kCheck:
      if(!reg.isPending()){
          if(data == reg.getData()){
              reg_configured = true;
              reg.setConfigured();
              state = State::kInit;
          }else{
              state = State::kWrite;
          }
      }
      break;
          
  }
  return reg_configured;
}

Refresher

#include "Register.h"

class Refresher
{
public:
  Refresher();
  bool refresh(Register &reg);

private:
  enum class State {kRead, kCheck};

  State state;
};

#include "Refresher.h"
    
Refresher::Refresher()
{
    state = State::kRead;
}

bool Refresher::refresh(Register &reg)
{
   bool reg_refreshed = false; 
    
   switch(state){
       case State::kRead:
            if(reg.read()){
                reg.setUnRefreshed();
                state = State::kCheck;
            }
           break;
       case State::kCheck:
           if(!reg.isPending()){
               if(reg.isRefreshed()){
                   reg_refreshed = true;
               }
               state = State::kRead;
           }
           break;
   }    
   return reg_refreshed;
}

Message

class Message
{
  public:
    enum class MessageType
    {
        kWriteRegRequest,
        kReadRegRequest,
        kWriteRegResponse,
        kReadRegResponse
    };  
      
    virtual void read(void) = 0;
    virtual void write(void) = 0;
};

WriteRegRequest

#include "Message.h"
#include "DigitalInputsDriverCfg.h"
#include "Transceiver.h"

class WriteRegRequestMsg : public Message
{
  public:
    WriteRegRequestMsg();
    WriteRegRequestMsg(DigitalInputsDriverCfg::Device _device, uint8_t _address,
                       uint8_t _data, Transceiver *_transceiver);
    void read(void);
    void write(void);

  private:
    union MsgData{
      struct
      {
        uint32_t crc : 5;  
        uint32_t fill : 3;
        uint32_t data : 8;
        uint32_t reg_addr : 7;
        uint32_t msb : 1;
      }bits;
      uint8_t bytes[3];
    };

    DigitalInputsDriverCfg::Device device;
    uint8_t address;
    uint8_t data;
    MsgData msg;
    uint8_t length;
    Transceiver *transceiver;
  };

#include "WriteRegRequestMsg.h"

WriteRegRequestMsg::WriteRegRequestMsg(){}
WriteRegRequestMsg::WriteRegRequestMsg(
    DigitalInputsDriverCfg::Device _device, uint8_t _address, uint8_t _data,
    Transceiver *_transceiver)
{
  device = _device;
  address = _address;
  data = _data;
  transceiver = _transceiver;
}

void WriteRegRequestMsg::read(void){}
void WriteRegRequestMsg::write(void)
{
  msg.bits.msb = 1;
  msg.bits.reg_addr = (address & 0x7F);
  msg.bits.data = data;
  msg.bits.fill = 0;
  uint8_t crc = crcMAX22190(msg.bytes[2], msg.bytes[1], msg.bytes[0]);
  msg.bits.crc = crc;
  
  transceiver->putBytes(msg.bytes);
  transceiver->startTransaction(device);
}

ReadRegRequest

#include "Message.h"
#include "DigitalInputsDriverCfg.h"
#include "Transceiver.h"

class ReadRegRequestMsg : public Message
{
  public:
    ReadRegRequestMsg();
    ReadRegRequestMsg(DigitalInputsDriverCfg::Device _device, uint8_t _address, Transceiver *_transceiver);
    void read(void);
    void write(void);

  private:
    union MsgData{
      struct
      {
        uint32_t crc : 5;
        uint32_t fill : 11;
        uint32_t reg_addr : 7;
        uint32_t msb : 1;
      }bits;
      uint8_t bytes[3];
    };

    DigitalInputsDriverCfg::Device device;
    uint8_t address;
    MsgData msg;
    Transceiver *transceiver;
};

#include "ReadRegRequestMsg.h"

ReadRegRequestMsg::ReadRegRequestMsg(){}
ReadRegRequestMsg::ReadRegRequestMsg(DigitalInputsDriverCfg::Device _device,
                                               uint8_t _address,
                                               Transceiver *_transceiver)
{
  device = _device;
  address = _address;
  transceiver = _transceiver;
}

void ReadRegRequestMsg::read(void){}
void ReadRegRequestMsg::write(void)
{
  msg.bits.msb = 0;
  msg.bits.reg_addr = address;
  msg.bits.fill = 0;
  uint8_t crc = crcMAX22190(msg.bytes[2], msg.bytes[1], msg.bytes[0]);
  msg.bits.crc = crc;
  
  transceiver->putBytes(msg.bytes);
  transceiver->startTransaction(device);
}

WriteRegResponse

#include "Message.h"
#include "Transceiver.h"

class WriteRegResponseMsg : public Message
{
  public:
    WriteRegResponseMsg();
    WriteRegResponseMsg(Transceiver *_transceiver);
    void read(void);
    void write(void);
    bool isValid(void);
    uint32_t getData(void);

  private:
    union MsgData{
      struct
      {
        uint32_t crc : 5;
        uint32_t flags : 3;
        uint32_t wb_reg_state : 8;
        uint32_t inputs_state : 8;        
      }bits;
      uint8_t bytes[3];
    };
    MsgData msg;
    Transceiver *transceiver;
  };

#include "WriteRegResponseMsg.h"

WriteRegResponseMsg::WriteRegResponseMsg(){}
WriteRegResponseMsg::WriteRegResponseMsg(Transceiver *_transceiver)
{
  transceiver = _transceiver;
}

void WriteRegResponseMsg::read(void)
{
  transceiver->getBytes(msg.bytes);
}

void WriteRegResponseMsg::write(void){}
bool WriteRegResponseMsg::isValid(void)
{
  uint8_t crc = crcMAX22190(msg.bytes[2], msg.bytes[1], msg.bytes[0]);
  if(crc == 0){
    return true;
  }else{
    return false;
  }
}

uint32_t WriteRegResponseMsg::getData(void)
{
  uint32_t data = 0;
  data = ((static_cast<uint32_t>(msg.bytes[0]) << 16) |
          (static_cast<uint32_t>(msg.bytes[1]) << 8)  |
          (static_cast<uint32_t>(msg.bytes[2]) & 0xE0));
  return data;
}

ReadRegResponse

#include "Message.h"
#include "Transceiver.h"

class ReadRegResponseMsg : public Message
{
  public:
    ReadRegResponseMsg();
    ReadRegResponseMsg(Transceiver *_transceiver);
    void read(void);
    void write(void);
    bool isValid(void);
    uint32_t getData(void);
    uint8_t getRegisterData(void);

  private:
    union MsgData{
      struct
      {
        uint32_t crc : 5;
        uint32_t flags : 3;
        uint32_t reg_state : 8;
        uint32_t inputs_state : 8;
      }bits;
      uint8_t bytes[3];
    };
    MsgData msg;
    Transceiver *transceiver;
};

#include "ReadRegResponseMsg.h"

ReadRegResponseMsg::ReadRegResponseMsg() {}
ReadRegResponseMsg::ReadRegResponseMsg(Transceiver *_transceiver)
{
  transceiver = _transceiver;
}

void ReadRegResponseMsg::read(void)
{
  transceiver->getBytes(msg.bytes);
}

void ReadRegResponseMsg::write(void){}
bool ReadRegResponseMsg::isValid(void)
{
  uint8_t crc = crcMAX22190(msg.bytes[2], msg.bytes[1], msg.bytes[0]);
  if(crc == 0){
    return true;
  }else{
    return false;
  }
}

uint32_t ReadRegResponseMsg::getData(void)
{
  uint32_t data = 0;
  data = ((static_cast<uint32_t>(msg.bytes[0]) << 16) |
          (static_cast<uint32_t>(msg.bytes[1]) << 8) |
          (static_cast<uint32_t>(msg.bytes[2]) & 0xE0));
  return data;
}

uint8_t ReadRegResponseMsg::getRegisterData(void)
{
    return msg.bytes[1];
}

DigitalInputsDriverCfg

#include <cstdint>

class DigitalInputsDriverCfg
{
public:
  enum class Device {
    kMAX22190Device_0,
    kMAX22190Device_1,
    kNoMAX22190Devices
  };

  enum class Input{
    kInput_01,
    kInput_02,
    kInput_03,
    kInput_04,
    kInput_05,
    kInput_06,
    kInput_07,
    kInput_08,
    kNoInputs
  };

  enum class MAX22190RegisterType{
    kWb,
    kDi,
    kFault1,
    kFlt1,
    kFlt2,
    kFlt3,
    kFlt4,
    kFlt5,
    kFlt6,
    kFlt7,
    kFlt8,
    kCfg,
    kInEn,
    kFault2,
    kFault2En,
    kGpo,
    kFault1En,
    kNop
  };

  enum class WireBreakDetection{
    kWireBreakDetectionDisabled,
    kWireBreakDetectionEnabled
  };

  enum class ProgrammableFilter{
    kProgrammableFilterUsed,
    kProgrammableFilterBypassed
  };

  enum class FilterDelay{
    kInputFilterDelay50us,
    kInputFilterDelay100us,
    kInputFilterDelay400us,
    kInputFilterDelay800us,
    kInputFilterDelay1_6ms,
    kInputFilterDelay3_2ms,
    kInputFilterDelay12_8ms,
    kInputFilterDelay20ms
  };

  enum class Flags24VClearMethod{
    k24VFlagClearedBySpiTransactionFault1RegReading,
    k24VFlagClearedByFault1RegReading
  };

  enum class FiltersOperation{
    kFiltersOperationNormal,
    kFiltersOperationFixed
  };

  enum class ShortCircuitDetection{
    kShortCircuitDetectionDisabled,
    kShortCircuitDetectionEnabled
  };

  enum class InputEnable{kInputDisabled, kInputEnabled};

  enum class Fault2SrcInFault1Reg{
    kFault8CkeInFault1Reg,
    kOtShdnInFault1Reg,
    kPinREFDIOpenInFault1Reg,
    kPinREFDIShortInFault1Reg,
    kPinREFWBOpenInFault1Reg,
    kPinREFWBShortInFault1Reg,
    kNoFault2Src
  };

  enum class Fault2SrcUsage{kFault2SrcNotUsed, kFault2SrcUsed};

  struct Fault2BitInFault1RegCfg
  {
    Fault2SrcInFault1Reg fault2;
    Fault2SrcUsage usage;
  };

  enum class FaultPinCfg{kFaultPinNotSticky, kFaultPinSticky};

  enum class FaultPinActivationSrc{
    kFaultPinActivationCrc,
    kFaultPinActivationPor,
    kFaultPinActivationFault2,
    kFaultPinActivationAlarmT2,
    kFaultPinActivationAlarmT1,
    kFaultPinActivation24VL,
    kFaultPinActivation24VM,
    kFaultPinActivationWireBreak,
    kNoFaultPinActivationSrc
  };

  enum class FaultPinActivationSrcUsage{kNotUsed, kUsed};

  struct FaultPinActivationCfg
  {
    FaultPinActivationSrc src;
    FaultPinActivationSrcUsage usage;
  };

  struct InputCfg
  {
    Input input;
    InputEnable input_enable;
    WireBreakDetection wire_break_detection_enable;
    ProgrammableFilter filter_enable;
    FilterDelay filter_delay;
  };

  struct MAX22190Config
  {
    Device device;
    InputCfg inputs_cfg[static_cast<uint8_t>(Input::kNoInputs)];
    Flags24VClearMethod flags_clear_method;
    FiltersOperation filters_operation;
    ShortCircuitDetection short_circuit_detection;
    Fault2BitInFault1RegCfg fault2_src_cfg[static_cast<uint8_t>(
        Fault2SrcInFault1Reg::kNoFault2Src)];
    FaultPinCfg fault_pin_cfg;
    FaultPinActivationCfg fault_pin_activation_cfg[static_cast<uint8_t>(
        FaultPinActivationSrc::kNoFaultPinActivationSrc)];
  };

  DigitalInputsDriverCfg(const MAX22190Config *_configuration);
  uint8_t getRegisterConfigData(Device device, MAX22190RegisterType reg);

private:
  union Fault1RegBitMap{
    struct
    {
      uint32_t wbg_bit : 1;
      uint32_t _24vm_bit : 1;
      uint32_t _24vl_bit : 1;
      uint32_t alarmt1_bit : 1;
      uint32_t alarmt2_bit : 1;
      uint32_t fault2_bit : 1;
      uint32_t por_bit : 1;
      uint32_t crc_bit : 1;
    }bits;
    uint8_t byte;
  };

  union FltxRegBitMap{
    struct
    {
      uint32_t delay : 3;
      uint32_t fbp_bit : 1;
      uint32_t wbe_bit : 1;
      uint32_t reserve : 3;
    }bits;
    uint8_t byte;
  };

  union CfgRegBitMap{
    struct
    {
      uint32_t refdi_sh_ena_bit : 1;
      uint32_t reserve_01 : 2;
      uint32_t clrf_bit : 1;
      uint32_t _24vf_bit : 1;
      uint32_t reserve_02 : 3;
    }bits;
    uint8_t byte;
  };

  union Fault2RegBitMap{
    struct
    {
      uint32_t rfwbs_bit : 1;
      uint32_t rfwbo_bit : 1;
      uint32_t rfdis_bit : 1;
      uint32_t rfdio_bit : 1;
      uint32_t otshdn_bit : 1;
      uint32_t fault8ck_bit : 1;
      uint32_t reserve : 2;
    }bits;
    uint8_t byte;
  };

  union Fault2EnRegBitMap{
    struct
    {
      uint32_t rfwbse_bit : 1;
      uint32_t rfwboe_bit : 1;
      uint32_t rfdise_bit : 1;
      uint32_t rfdioe_bit : 1;
      uint32_t otshdne_bit : 1;
      uint32_t fault8cke_bit : 1;
      uint32_t reserve : 2;
    }bits;
    uint8_t byte;
  };

  union GpoRegBitMap{
    struct
    {
      uint32_t reserve : 7;
      uint32_t stk_bit : 1;
    }bits;
    uint8_t byte;
  };

  union Fault1EnRegBitMap{
    struct
    {
      uint32_t wbge_bit : 1;
      uint32_t _24vme_bit : 1;
      uint32_t _24vle_bit : 1;
      uint32_t alarmt1e_bit : 1;
      uint32_t alarmt2e_bit : 1;
      uint32_t fault2e_bit : 1;
      uint32_t pore_bit : 1;
      uint32_t crce_bit : 1;
    }bits;
    uint8_t byte;
  };

  const MAX22190Config *configuration;

  uint8_t readInEnRegCfgData(Device device);
  uint8_t readFltxRegCfgData(Device device, Input input);
  uint8_t readFault2EnRegCfgData(Device device);
  uint8_t readFault1EnRegCfgData(Device device);
  uint8_t readCfgRegCfgData(Device device);
  uint8_t readGpoRegCfgData(Device device);
};

#include "DigitalInputsDriverCfg.h"
   
DigitalInputsDriverCfg::DigitalInputsDriverCfg(const MAX22190Config* _configuration)
{
  configuration = _configuration;
}

uint8_t DigitalInputsDriverCfg::getRegisterConfigData(Device device, MAX22190RegisterType reg)
{
  uint8_t data;
  switch(reg)
  {
    case MAX22190RegisterType::kWb:
      data = 0;
    break;
    case MAX22190RegisterType::kDi:
      data = 0;
    break;
    case MAX22190RegisterType::kFault1:
      data = 0;
    break;
    case MAX22190RegisterType::kFlt1:
      data = readFltxRegCfgData(device, Input::kInput_01);
    break;
    case MAX22190RegisterType::kFlt2:
      data = readFltxRegCfgData(device, Input::kInput_02);
    break;
    case MAX22190RegisterType::kFlt3:
      data = readFltxRegCfgData(device, Input::kInput_03);
    break;
    case MAX22190RegisterType::kFlt4:
      data = readFltxRegCfgData(device, Input::kInput_04);
    break;
    case MAX22190RegisterType::kFlt5:
      data = readFltxRegCfgData(device, Input::kInput_05);
    break;
    case MAX22190RegisterType::kFlt6:
      data = readFltxRegCfgData(device, Input::kInput_06);
    break;
    case MAX22190RegisterType::kFlt7:
      data = readFltxRegCfgData(device, Input::kInput_07);
    break;
    case MAX22190RegisterType::kFlt8:
      data = readFltxRegCfgData(device, Input::kInput_08);
    break;
    case MAX22190RegisterType::kCfg:
      data = readCfgRegCfgData(device);
    break;
    case MAX22190RegisterType::kInEn:
      data = readInEnRegCfgData(device);
    break;
    case MAX22190RegisterType::kFault2:
      data = 0;
    break;
    case MAX22190RegisterType::kFault2En:
      data = readFault2EnRegCfgData(device);
    break;
    case MAX22190RegisterType::kGpo:
      data = readGpoRegCfgData(device);
    break;
    case MAX22190RegisterType::kFault1En:
      data = readFault1EnRegCfgData(device);
    break;
    case MAX22190RegisterType::kNop:
      data = 0;
    break;
  }
  return data;
}

uint8_t DigitalInputsDriverCfg::readInEnRegCfgData(Device device)
{
  uint8_t reg_data = 0;

  for(uint8_t cur_input = 0;
       cur_input < static_cast<uint8_t>(Input::kNoInputs);
       cur_input++){

    if(configuration[static_cast<uint8_t>(device)]
            .inputs_cfg[cur_input]
            .input_enable == InputEnable::kInputEnabled){
      reg_data |= (1 << cur_input);
    }
  }
  return reg_data;
}

uint8_t DigitalInputsDriverCfg::readFltxRegCfgData(Device device, Input input)
{
  FltxRegBitMap reg_data;
  reg_data.byte = 0;

  if(configuration[static_cast<uint8_t>(device)]
          .inputs_cfg[static_cast<uint8_t>(input)]
          .wire_break_detection_enable ==
      WireBreakDetection::kWireBreakDetectionEnabled){
    reg_data.bits.wbe_bit = 1;
  }

  if(configuration[static_cast<uint8_t>(device)]
          .inputs_cfg[static_cast<uint8_t>(input)]
          .filter_enable ==
      ProgrammableFilter::kProgrammableFilterBypassed){
    reg_data.bits.fbp_bit = 1;
  }

  reg_data.bits.delay =
      static_cast<uint8_t>(configuration[static_cast<uint8_t>(device)]
                               .inputs_cfg[static_cast<uint8_t>(input)]
                               .filter_delay);
  return reg_data.byte;
}

uint8_t DigitalInputsDriverCfg::readFault2EnRegCfgData(Device device)
{
  Fault2EnRegBitMap reg_data;
  reg_data.byte = 0;

  for(uint8_t cur_record = 0;
       cur_record < static_cast<uint8_t>(
                        Fault2SrcInFault1Reg::kNoFault2Src);
       cur_record++){
    switch (configuration[static_cast<uint8_t>(device)]
                .fault2_src_cfg[cur_record]
                .fault2){

      case Fault2SrcInFault1Reg::kFault8CkeInFault1Reg:
        if(configuration[static_cast<uint8_t>(device)]
                .fault2_src_cfg[cur_record]
                .usage == Fault2SrcUsage::kFault2SrcUsed){
          reg_data.bits.fault8cke_bit = 1;
        }
        break;
      case Fault2SrcInFault1Reg::kOtShdnInFault1Reg:
        if(configuration[static_cast<uint8_t>(device)]
                .fault2_src_cfg[cur_record]
                .usage == Fault2SrcUsage::kFault2SrcUsed){
          reg_data.bits.otshdne_bit = 1;
        }
        break;
      case Fault2SrcInFault1Reg::kPinREFDIOpenInFault1Reg:
        if(configuration[static_cast<uint8_t>(device)]
                .fault2_src_cfg[cur_record]
                .usage == Fault2SrcUsage::kFault2SrcUsed){
          reg_data.bits.rfdioe_bit = 1;
        }
        break;
      case Fault2SrcInFault1Reg::kPinREFDIShortInFault1Reg:
        if(configuration[static_cast<uint8_t>(device)]
                .fault2_src_cfg[cur_record]
                .usage == Fault2SrcUsage::kFault2SrcUsed){
          reg_data.bits.rfdise_bit = 1;
        }
        break;
      case Fault2SrcInFault1Reg::kPinREFWBOpenInFault1Reg:
        if(configuration[static_cast<uint8_t>(device)]
                .fault2_src_cfg[cur_record]
                .usage == Fault2SrcUsage::kFault2SrcUsed){
          reg_data.bits.rfwboe_bit = 1;
        }
        break;
      case Fault2SrcInFault1Reg::kPinREFWBShortInFault1Reg:
        if(configuration[static_cast<uint8_t>(device)]
                .fault2_src_cfg[cur_record]
                .usage == Fault2SrcUsage::kFault2SrcUsed){
          reg_data.bits.rfwbse_bit = 1;
        }
        break;
    }
  }
  return reg_data.byte;
}

uint8_t DigitalInputsDriverCfg::readFault1EnRegCfgData(Device device)
{
  Fault1EnRegBitMap reg_data;
  reg_data.byte = 0;

  for(uint8_t cur_record = 0;
       cur_record <
       static_cast<uint8_t>(
           FaultPinActivationSrc::kNoFaultPinActivationSrc);
       cur_record++){

    switch(configuration[static_cast<uint8_t>(device)]
                .fault_pin_activation_cfg[cur_record]
                .src){

      case FaultPinActivationSrc::kFaultPinActivationCrc:
        if(configuration[static_cast<uint8_t>(device)]
                .fault_pin_activation_cfg[cur_record]
                .usage == FaultPinActivationSrcUsage::kUsed){
          reg_data.bits.crce_bit = 1;
        }
        break;

      case FaultPinActivationSrc::kFaultPinActivationPor:
        if(configuration[static_cast<uint8_t>(device)]
                .fault_pin_activation_cfg[cur_record]
                .usage == FaultPinActivationSrcUsage::kUsed){
          reg_data.bits.pore_bit = 1;
        }
        break;

      case FaultPinActivationSrc::kFaultPinActivationFault2:
        if(configuration[static_cast<uint8_t>(device)]
                .fault_pin_activation_cfg[cur_record]
                .usage == FaultPinActivationSrcUsage::kUsed){
          reg_data.bits.fault2e_bit = 1;
        }
        break;

      case FaultPinActivationSrc::kFaultPinActivationAlarmT2:
        if(configuration[static_cast<uint8_t>(device)]
                .fault_pin_activation_cfg[cur_record]
                .usage == FaultPinActivationSrcUsage::kUsed){
          reg_data.bits.alarmt2e_bit = 1;
        }
        break;

      case FaultPinActivationSrc::kFaultPinActivationAlarmT1:
        if(configuration[static_cast<uint8_t>(device)]
                .fault_pin_activation_cfg[cur_record]
                .usage == FaultPinActivationSrcUsage::kUsed){
          reg_data.bits.alarmt1e_bit = 1;
        }
        break;

      case FaultPinActivationSrc::kFaultPinActivation24VL:
        if(configuration[static_cast<uint8_t>(device)]
                .fault_pin_activation_cfg[cur_record]
                .usage == FaultPinActivationSrcUsage::kUsed){
          reg_data.bits._24vle_bit = 1;
        }
        break;

      case FaultPinActivationSrc::kFaultPinActivation24VM:
        if(configuration[static_cast<uint8_t>(device)]
                .fault_pin_activation_cfg[cur_record]
                .usage == FaultPinActivationSrcUsage::kUsed){
          reg_data.bits._24vme_bit = 1;
        }
        break;

      case FaultPinActivationSrc::
          kFaultPinActivationWireBreak:
        if(configuration[static_cast<uint8_t>(device)]
                .fault_pin_activation_cfg[cur_record]
                .usage == FaultPinActivationSrcUsage::kUsed){
          reg_data.bits.wbge_bit = 1;
        }
        break;
    }
  }
  return reg_data.byte;
}

uint8_t DigitalInputsDriverCfg::readCfgRegCfgData(Device device)
{
  CfgRegBitMap reg_data;
  reg_data.byte = 0;

  if(configuration[static_cast<uint8_t>(device)].short_circuit_detection ==
      ShortCircuitDetection::kShortCircuitDetectionEnabled){
    reg_data.bits.refdi_sh_ena_bit = 1;
  }

  if(configuration[static_cast<uint8_t>(device)].filters_operation ==
      FiltersOperation::kFiltersOperationFixed){
    reg_data.bits.clrf_bit = 1;
  }

  if(configuration[static_cast<uint8_t>(device)].flags_clear_method ==
      Flags24VClearMethod::
          k24VFlagClearedByFault1RegReading){
    reg_data.bits._24vf_bit = 1;
  }
  return reg_data.byte;
}

uint8_t DigitalInputsDriverCfg::readGpoRegCfgData(Device device)
{
  GpoRegBitMap reg_data;
  reg_data.byte = 0;

  if(configuration[static_cast<uint8_t>(device)].fault_pin_cfg ==
      FaultPinCfg::kFaultPinSticky){
    reg_data.bits.stk_bit = 1;
  }
  return reg_data.byte;
}

The intended usage of the driver is following:

  • whole the configuration information are in the FpgaDriversCfg struct
  • the instance of the DigitalInputsDriver is along with instances of other drivers in the "container" Hal

FpgaDriversCfg

#include "DigitalInputsDriverCfg.h"
    
struct FpgaDriversCfg 
{            
    DigitalInputsDriverCfg::MAX22190Config di_devices_cfg[static_cast<uint8_t>(
      DigitalInputsDriverCfg::Device::kNoMAX22190Devices)]={   
      {DigitalInputsDriverCfg::Device::kMAX22190Device_0,
       {{DigitalInputsDriverCfg::Input::kInput_01,
         DigitalInputsDriverCfg::InputEnable::kInputEnabled,
         DigitalInputsDriverCfg::WireBreakDetection::kWireBreakDetectionEnabled,
         DigitalInputsDriverCfg::ProgrammableFilter::kProgrammableFilterUsed,
         DigitalInputsDriverCfg::FilterDelay::kInputFilterDelay20ms},
        {DigitalInputsDriverCfg::Input::kInput_02,
         DigitalInputsDriverCfg::InputEnable::kInputEnabled,
         DigitalInputsDriverCfg::WireBreakDetection::kWireBreakDetectionEnabled,
         DigitalInputsDriverCfg::ProgrammableFilter::kProgrammableFilterUsed,
         DigitalInputsDriverCfg::FilterDelay::kInputFilterDelay20ms},
        {DigitalInputsDriverCfg::Input::kInput_03,
         DigitalInputsDriverCfg::InputEnable::kInputEnabled,
         DigitalInputsDriverCfg::WireBreakDetection::kWireBreakDetectionEnabled,
         DigitalInputsDriverCfg::ProgrammableFilter::kProgrammableFilterUsed,
         DigitalInputsDriverCfg::FilterDelay::kInputFilterDelay20ms},
        {DigitalInputsDriverCfg::Input::kInput_04,
         DigitalInputsDriverCfg::InputEnable::kInputEnabled,
         DigitalInputsDriverCfg::WireBreakDetection::kWireBreakDetectionEnabled,
         DigitalInputsDriverCfg::ProgrammableFilter::kProgrammableFilterUsed,
         DigitalInputsDriverCfg::FilterDelay::kInputFilterDelay20ms},
        {DigitalInputsDriverCfg::Input::kInput_05,
         DigitalInputsDriverCfg::InputEnable::kInputEnabled,
         DigitalInputsDriverCfg::WireBreakDetection::kWireBreakDetectionEnabled,
         DigitalInputsDriverCfg::ProgrammableFilter::kProgrammableFilterUsed,
         DigitalInputsDriverCfg::FilterDelay::kInputFilterDelay20ms},
        {DigitalInputsDriverCfg::Input::kInput_06,
         DigitalInputsDriverCfg::InputEnable::kInputEnabled,
         DigitalInputsDriverCfg::WireBreakDetection::kWireBreakDetectionEnabled,
         DigitalInputsDriverCfg::ProgrammableFilter::kProgrammableFilterUsed,
         DigitalInputsDriverCfg::FilterDelay::kInputFilterDelay20ms},
        {DigitalInputsDriverCfg::Input::kInput_07,
         DigitalInputsDriverCfg::InputEnable::kInputEnabled,
         DigitalInputsDriverCfg::WireBreakDetection::kWireBreakDetectionEnabled,
         DigitalInputsDriverCfg::ProgrammableFilter::kProgrammableFilterUsed,
         DigitalInputsDriverCfg::FilterDelay::kInputFilterDelay20ms},
        {DigitalInputsDriverCfg::Input::kInput_08,
         DigitalInputsDriverCfg::InputEnable::kInputEnabled,
         DigitalInputsDriverCfg::WireBreakDetection::kWireBreakDetectionEnabled,
         DigitalInputsDriverCfg::ProgrammableFilter::kProgrammableFilterUsed,
         DigitalInputsDriverCfg::FilterDelay::kInputFilterDelay20ms}},
       DigitalInputsDriverCfg::Flags24VClearMethod::
           k24VFlagClearedBySpiTransactionFault1RegReading,
       DigitalInputsDriverCfg::FiltersOperation::kFiltersOperationNormal,
       DigitalInputsDriverCfg::ShortCircuitDetection::kShortCircuitDetectionEnabled,
       {{DigitalInputsDriverCfg::Fault2SrcInFault1Reg::kFault8CkeInFault1Reg,
         DigitalInputsDriverCfg::Fault2SrcUsage::kFault2SrcUsed},
        {DigitalInputsDriverCfg::Fault2SrcInFault1Reg::kOtShdnInFault1Reg,
         DigitalInputsDriverCfg::Fault2SrcUsage::kFault2SrcUsed},
        {DigitalInputsDriverCfg::Fault2SrcInFault1Reg::kPinREFDIOpenInFault1Reg,
         DigitalInputsDriverCfg::Fault2SrcUsage::kFault2SrcUsed},
        {DigitalInputsDriverCfg::Fault2SrcInFault1Reg::kPinREFDIShortInFault1Reg,
         DigitalInputsDriverCfg::Fault2SrcUsage::kFault2SrcUsed},
        {DigitalInputsDriverCfg::Fault2SrcInFault1Reg::kPinREFWBOpenInFault1Reg,
         DigitalInputsDriverCfg::Fault2SrcUsage::kFault2SrcUsed},
        {DigitalInputsDriverCfg::Fault2SrcInFault1Reg::kPinREFWBShortInFault1Reg,
         DigitalInputsDriverCfg::Fault2SrcUsage::kFault2SrcUsed}},
       DigitalInputsDriverCfg::FaultPinCfg::kFaultPinSticky,
       {{DigitalInputsDriverCfg::FaultPinActivationSrc::kFaultPinActivationCrc,
         DigitalInputsDriverCfg::FaultPinActivationSrcUsage::kUsed},
        {DigitalInputsDriverCfg::FaultPinActivationSrc::kFaultPinActivationPor,
         DigitalInputsDriverCfg::FaultPinActivationSrcUsage::kUsed},
        {DigitalInputsDriverCfg::FaultPinActivationSrc::kFaultPinActivationFault2,
         DigitalInputsDriverCfg::FaultPinActivationSrcUsage::kUsed},
        {DigitalInputsDriverCfg::FaultPinActivationSrc::kFaultPinActivationAlarmT2,
         DigitalInputsDriverCfg::FaultPinActivationSrcUsage::kUsed},
        {DigitalInputsDriverCfg::FaultPinActivationSrc::kFaultPinActivationAlarmT1,
         DigitalInputsDriverCfg::FaultPinActivationSrcUsage::kUsed},
        {DigitalInputsDriverCfg::FaultPinActivationSrc::kFaultPinActivation24VL,
         DigitalInputsDriverCfg::FaultPinActivationSrcUsage::kUsed},
        {DigitalInputsDriverCfg::FaultPinActivationSrc::kFaultPinActivation24VM,
         DigitalInputsDriverCfg::FaultPinActivationSrcUsage::kUsed},
        {DigitalInputsDriverCfg::FaultPinActivationSrc::kFaultPinActivationWireBreak,
         DigitalInputsDriverCfg::FaultPinActivationSrcUsage::kUsed}}},
      {DigitalInputsDriverCfg::Device::kMAX22190Device_1,
       {{DigitalInputsDriverCfg::Input::kInput_01,
         DigitalInputsDriverCfg::InputEnable::kInputEnabled,
         DigitalInputsDriverCfg::WireBreakDetection::kWireBreakDetectionEnabled,
         DigitalInputsDriverCfg::ProgrammableFilter::kProgrammableFilterUsed,
         DigitalInputsDriverCfg::FilterDelay::kInputFilterDelay20ms},
        {DigitalInputsDriverCfg::Input::kInput_02,
         DigitalInputsDriverCfg::InputEnable::kInputEnabled,
         DigitalInputsDriverCfg::WireBreakDetection::kWireBreakDetectionEnabled,
         DigitalInputsDriverCfg::ProgrammableFilter::kProgrammableFilterUsed,
         DigitalInputsDriverCfg::FilterDelay::kInputFilterDelay20ms},
        {DigitalInputsDriverCfg::Input::kInput_03,
         DigitalInputsDriverCfg::InputEnable::kInputEnabled,
         DigitalInputsDriverCfg::WireBreakDetection::kWireBreakDetectionEnabled,
         DigitalInputsDriverCfg::ProgrammableFilter::kProgrammableFilterUsed,
         DigitalInputsDriverCfg::FilterDelay::kInputFilterDelay20ms},
        {DigitalInputsDriverCfg::Input::kInput_04,
         DigitalInputsDriverCfg::InputEnable::kInputEnabled,
         DigitalInputsDriverCfg::WireBreakDetection::kWireBreakDetectionEnabled,
         DigitalInputsDriverCfg::ProgrammableFilter::kProgrammableFilterUsed,
         DigitalInputsDriverCfg::FilterDelay::kInputFilterDelay20ms},
        {DigitalInputsDriverCfg::Input::kInput_05,
         DigitalInputsDriverCfg::InputEnable::kInputEnabled,
         DigitalInputsDriverCfg::WireBreakDetection::kWireBreakDetectionEnabled,
         DigitalInputsDriverCfg::ProgrammableFilter::kProgrammableFilterUsed,
         DigitalInputsDriverCfg::FilterDelay::kInputFilterDelay20ms},
        {DigitalInputsDriverCfg::Input::kInput_06,
         DigitalInputsDriverCfg::InputEnable::kInputEnabled,
         DigitalInputsDriverCfg::WireBreakDetection::kWireBreakDetectionEnabled,
         DigitalInputsDriverCfg::ProgrammableFilter::kProgrammableFilterUsed,
         DigitalInputsDriverCfg::FilterDelay::kInputFilterDelay20ms},
        {DigitalInputsDriverCfg::Input::kInput_07,
         DigitalInputsDriverCfg::InputEnable::kInputEnabled,
         DigitalInputsDriverCfg::WireBreakDetection::kWireBreakDetectionEnabled,
         DigitalInputsDriverCfg::ProgrammableFilter::kProgrammableFilterUsed,
         DigitalInputsDriverCfg::FilterDelay::kInputFilterDelay20ms},
        {DigitalInputsDriverCfg::Input::kInput_08,
         DigitalInputsDriverCfg::InputEnable::kInputEnabled,
         DigitalInputsDriverCfg::WireBreakDetection::kWireBreakDetectionEnabled,
         DigitalInputsDriverCfg::ProgrammableFilter::kProgrammableFilterUsed,
         DigitalInputsDriverCfg::FilterDelay::kInputFilterDelay20ms}},
       DigitalInputsDriverCfg::Flags24VClearMethod::
           k24VFlagClearedBySpiTransactionFault1RegReading,
       DigitalInputsDriverCfg::FiltersOperation::kFiltersOperationNormal,
       DigitalInputsDriverCfg::ShortCircuitDetection::kShortCircuitDetectionEnabled,
       {{DigitalInputsDriverCfg::Fault2SrcInFault1Reg::kFault8CkeInFault1Reg,
         DigitalInputsDriverCfg::Fault2SrcUsage::kFault2SrcUsed},
        {DigitalInputsDriverCfg::Fault2SrcInFault1Reg::kOtShdnInFault1Reg,
         DigitalInputsDriverCfg::Fault2SrcUsage::kFault2SrcUsed},
        {DigitalInputsDriverCfg::Fault2SrcInFault1Reg::kPinREFDIOpenInFault1Reg,
         DigitalInputsDriverCfg::Fault2SrcUsage::kFault2SrcUsed},
        {DigitalInputsDriverCfg::Fault2SrcInFault1Reg::kPinREFDIShortInFault1Reg,
         DigitalInputsDriverCfg::Fault2SrcUsage::kFault2SrcUsed},
        {DigitalInputsDriverCfg::Fault2SrcInFault1Reg::kPinREFWBOpenInFault1Reg,
         DigitalInputsDriverCfg::Fault2SrcUsage::kFault2SrcUsed},
        {DigitalInputsDriverCfg::Fault2SrcInFault1Reg::kPinREFWBShortInFault1Reg,
         DigitalInputsDriverCfg::Fault2SrcUsage::kFault2SrcUsed}},
       DigitalInputsDriverCfg::FaultPinCfg::kFaultPinSticky,
       {{DigitalInputsDriverCfg::FaultPinActivationSrc::kFaultPinActivationCrc,
         DigitalInputsDriverCfg::FaultPinActivationSrcUsage::kUsed},
        {DigitalInputsDriverCfg::FaultPinActivationSrc::kFaultPinActivationPor,
         DigitalInputsDriverCfg::FaultPinActivationSrcUsage::kUsed},
        {DigitalInputsDriverCfg::FaultPinActivationSrc::kFaultPinActivationFault2,
         DigitalInputsDriverCfg::FaultPinActivationSrcUsage::kUsed},
        {DigitalInputsDriverCfg::FaultPinActivationSrc::kFaultPinActivationAlarmT2,
         DigitalInputsDriverCfg::FaultPinActivationSrcUsage::kUsed},
        {DigitalInputsDriverCfg::FaultPinActivationSrc::kFaultPinActivationAlarmT1,
         DigitalInputsDriverCfg::FaultPinActivationSrcUsage::kUsed},
        {DigitalInputsDriverCfg::FaultPinActivationSrc::kFaultPinActivation24VL,
         DigitalInputsDriverCfg::FaultPinActivationSrcUsage::kUsed},
        {DigitalInputsDriverCfg::FaultPinActivationSrc::kFaultPinActivation24VM,
         DigitalInputsDriverCfg::FaultPinActivationSrcUsage::kUsed},
        {DigitalInputsDriverCfg::FaultPinActivationSrc::kFaultPinActivationWireBreak,
         DigitalInputsDriverCfg::FaultPinActivationSrcUsage::kUsed}}}};

         DigitalInputsDriverCfg dig_inputs_cfg;

        FpgaDriversCfg():dig_inputs_cfg(di_devices_cfg){}
};

Hal

#include "FpgaDriversCfg.h"
#include "DigitalInputsDriver.h"

class Hal{
public:
    DigitalInputsDriver  dig_in;
        
    Hal();
    void initialize(void);
        
private:
    FpgaDriversCfg fpga_configuration;
};

#include "Hal.h"
#include "FpgaDriversCfg.h"
#include "Registers.h"
    
Hal::Hal():
    dig_in(&(fpga_configuration.dig_inputs_cfg), 1){}

void Hal::initialize(void){
    dig_in.initialize();
}

main

#include <cstdlib>
#include "Hal.h"
            
int main(int argc, char** argv){
         
    Hal hal;
    hal.initialize();
    
    while(1){
      hal.dig_in.update();
      // the methods getInputState, isFault1Active, isFault2Active are intended to be called from 
      // different tasks of the RTOS
    }        
}

I would like to ask you mainly for assessment of the whole driver design i.e. how the driver is divided into the C++ classes and how those classes interact. Further I have been thinking about the possibility to use some design pattern for working with the Register objects. Thank you in advance for any notes.

\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

Add include guards

I am not seeing any include guards in your code. It is common practice to add those to header files; this allows them to depend on other header files without having to worry about loops and duplicates. The simplest solution is to add the following to the top of each header file:

#pragma once

This is supported by most compilers, however it is not standard C++. The standards-compliant way is to write a header file like so:

#ifndef HEADERFILENAME_H
#define HEADERFILENAME_H

// Contents of header file come here
...

#endif

However, you need to replace HEADERFILENAME with something unique for each header file, typically the header's filename is used for that.

Use of enums

In principle using enum class for constants gives you excellent type safety, however there are some drawbacks, in particular there is the need to cast them whenever you need to get their integer value. I would keep using enum classes for things where you have really distinct names for each possible value, like the fault codes. However, I would not use them for pin numbers if you are just going to give the pin numbers names that have a one-to-one correspondence to their value, like in:

enum class Input {
    kDi_00,
    kDi_01,
    ...
};

Also, I see you prefixed all the enum names with a k. This is probably a leftover from a coding style using Hungarian notation, but since you don't use Hungarian notation for anything else, it doesn't make much sense. I would remove the ks, they just add visible noise and don't do anything to make the code safer.

Avoid starting identifiers with underscores

There are certain rules about using underscores in identifiers. In particular, starting with an underscore or using a double underscore is reserved in some situations. Your use is fine according to those rules, but if you were not aware of this to begin with, I would advise you to not start any names with underscores. I see you use those always in constructors, but they are almost never necessary. You only need them if you would shadow a member variable, and need to access both the member variable and the parameter at the same time in the body. However, note that you can use the same name as a member variable if you only use it in a constructor's initializer list, like so:

Register::Register(DigitalInputsDriverCfg::Device device,
                   DigitalInputsDriverCfg::MAX22190RegisterType type, uint8_t address,
                   Transceiver *transceiver) :
    device{device},
    type{type},
    address{address},
    transceiver{transceiver},
{...}    

Avoid the array of pointer in DigitalInputsDriver

There is no need to have both device0, device1 and an array devices[]. I assume the reason you have this is because you want to be able to initialize device0 and device1 in the constructor's initializer list, but wanted to be able to access them as an array. You can remove the redundancy and get the best of both worlds though, by writing it as follows:

class DigitalInputsDriver
{
    ...
    MAX22190 devices[2];
};

DigitalInputsDriver::DigitalInputsDriver(DigitalInputsDriverCfg *dig_in_cfg, uint16_t spi_device_id) :
    transceiver{spi_device_id},
    devices{
        {DigitalInputsDriverCfg::Device::kMAX22190Device_0, dig_in_cfg, &transceiver},
        {DigitalInputsDriverCfg::Device::kMAX22190Device_1, dig_in_cfg, &transceiver},
    }
{}         

You'll have to update all the code that uses devices[] now that it no longer contains pointers but values, for example:

void DigitalInputsDriver::update(void)
{
    for (MAX22190 &device : devices) {
        device.update();
    }
}

Make member functions that do not modify state const

Functions that do not modify state like member variables should be marked const, so the compiler can generate more optimal code. It will also make the compiler generate an error if you do accidentily write to a member variable from a const function. For example:

class MAX22190 : public TransactionEndListener
{
    ...
    bool isReady(void) const;
    ...
};

bool MAX22190::isReady(void) const
{
    return device_ready && device_configured;
}

Prefer member variable initialization where appropriate

Instead of initializing member variables in the constructor, you can sometimes also initialize them at the place where the member variables are declared. For example:

class Register
{
    ...
    bool configured = false; // or bool configured{false}; or bool configured{};
    bool pending = false;
    bool refreshed = false;
    ...
};

This is especially useful if you have multiple constructors in the class, since it then avoids a lot of repetition.

Use of placement new

I was a bit surprised to see the following code:

bool Register::read(void)
{
    if (!transceiver->isBusy()) {
        new (&read_reg_request) ReadRegRequestMsg(device, address, transceiver);
        ...

Placement new is something you do in rather rare circumstances where you either want to avoid default construction or where copy assignment is not possible. There is no problem copy-assigning a ReadRegRequestMsg, and since read_reg_request was already default constructed, there is no performance gain here. I would replace this line of code with:

read_reg_request = ReadRegRequestMsg(device, address, transceiver);

But upon further inspection, why store the request in a member variable in the first place? It is not used outside Register::read(), so it could just have been a local variable. But the only thing you do is call write() on it, so you don't even need to store the result in a local variable, and can just write:

bool Register::read(void)
{
    if (!transceiver->isBusy()) {
        ReadRegRequestMsg(device, address, transceiver).write();
        ...

But then that brings me to:

Don't overcomplicate things

Why is ReadRegRequestMsg a class, why does it inherit from class Message which itself doesn't do anything useful, when the only thing you need is the function that writes a message to a device? It could be a stand-alone function:

void writeReadRegRequestMsg(DigitalInputsDriverCfg::Device device, uint8_t address, Transceiver *transceiver)
{
    union {
        struct
        {
            uint32_t crc : 5;
            uint32_t fill : 11;
            uint32_t reg_addr : 7;
            uint32_t msb : 1;
        } bits;
        uint8_t bytes[3];
    } msg;

    msg.bits.msb = 0;
    msg.bits.reg_addr = address;
    msg.bits.fill = 0;
    uint8_t crc = crcMAX22190(msg.bytes[2], msg.bytes[1], msg.bytes[0]);
    msg.bits.crc = crc;
  
    transceiver->putBytes(msg.bytes);
    transceiver->startTransaction(device);
}

Or perhaps better is to make this a member function of Register.

For-case loops

There are several parts of the code that look suspiciously like for-case-loops, like MAX22190::configure(), Configure::configure(), and Refresher::refresh(). Is this because you have modelled the system as a state machine, and want to advance the state one step at a time? The problem is that this makes the flow of the code hard to read. I would rather expect something like:

bool MAX22190::configure(void)
{
    for (auto reg: register_map) {
        configurator.configure(*reg));
    }

    return true;
}

And in turn have configurator.configure() do all the steps necessary to configure the register by itself.

\$\endgroup\$
2
\$\begingroup\$

Consistency

Starting with the enums - be consistent if you're going to number each one or not. Personally I prefer to not number them, and if I have constants (as returns from I2C, etc) then those are called out as constexpr values. Again, it's a matter of taste/preference which you go with, but you need to be consistent.

STL

Most of your for-loops are off on their own, and are trivial - this is very good, and using the STL would be overkill. However, there are a few places where the STL would be good.

/// In particular this
bool DigitalInputsDriver::isReady(void)
{
  bool retval = true;
  for(MAX22190 *device : devices){
    if(device->isReady() == false){
      retval = false;
      break;
    }
  }
  return retval;
}

/// could be this
bool DigitalInputsDriver::isReady(void)
{
  return std::all_of(begin(devices), end(devices), [](MAX22190 * device){
    return device->isReady();
  });
}

Suddenly your code is much more expressive of what you're trying to achieve.

const and [[nodiscard]]

You have a lot of functions for classes, but I don't see a lot of const or [[nodiscard]]. If a function doesn't modify the class, make sure that it's const- this particularly applies to the functions named isReady or isConfigured. Along those same lines, if you have a function that returns a value, it's probably an error if you don't read the result; indicating this to the compiler will help with a whole class of errors.

Core Guideline: Don’t declare a variable until you have a value to initialize it with

///In particular I'm looking at
uint32_t ReadRegResponseMsg::getData(void)
{
  uint32_t data = 0;
  data = ((static_cast<uint32_t>(msg.bytes[0]) << 16) |
          (static_cast<uint32_t>(msg.bytes[1]) << 8) |
          (static_cast<uint32_t>(msg.bytes[2]) & 0xE0));
  return data;
}
/// which could be
uint32_t ReadRegResponseMsg::getData(void)
{
  return ((static_cast<uint32_t>(msg.bytes[0]) << 16) |
          (static_cast<uint32_t>(msg.bytes[1]) << 8) |
          (static_cast<uint32_t>(msg.bytes[2]) & 0xE0));
}

You don't need to zero initialize data, and you may as well simply return instead of using a named variable. Not every compiler will catch that you override the value in data, but almost every compiler will try to do some fancy optimization like this. The same applies for the CRC functions.

Code Deduplication

ReadReg and WriteReg look nearly identical, so why not condense a lot of that into a single source file? Like in the message class that they derive from?

Then there's digitalInputsDriverCfg, which

uint8_t DigitalInputsDriverCfg::readFault2EnRegCfgData(Device device)
{
  Fault2EnRegBitMap reg_data;
  reg_data.byte = 0;

  for(uint8_t cur_record = 0;
       cur_record < static_cast<uint8_t>(
                        Fault2SrcInFault1Reg::kNoFault2Src);
       cur_record++){
    switch (configuration[static_cast<uint8_t>(device)]
                .fault2_src_cfg[cur_record]
                .fault2){

      case Fault2SrcInFault1Reg::kFault8CkeInFault1Reg:
        if(configuration[static_cast<uint8_t>(device)]
                .fault2_src_cfg[cur_record]
                .usage == Fault2SrcUsage::kFault2SrcUsed){
          reg_data.bits.fault8cke_bit = 1;
        }
        break;
      case Fault2SrcInFault1Reg::kOtShdnInFault1Reg:
        if(configuration[static_cast<uint8_t>(device)]
                .fault2_src_cfg[cur_record]
                .usage == Fault2SrcUsage::kFault2SrcUsed){
          reg_data.bits.otshdne_bit = 1;
        }
        break;
      case Fault2SrcInFault1Reg::kPinREFDIOpenInFault1Reg:
        if(configuration[static_cast<uint8_t>(device)]
                .fault2_src_cfg[cur_record]
                .usage == Fault2SrcUsage::kFault2SrcUsed){
          reg_data.bits.rfdioe_bit = 1;
        }
        break;
      case Fault2SrcInFault1Reg::kPinREFDIShortInFault1Reg:
        if(configuration[static_cast<uint8_t>(device)]
                .fault2_src_cfg[cur_record]
                .usage == Fault2SrcUsage::kFault2SrcUsed){
          reg_data.bits.rfdise_bit = 1;
        }
        break;
      case Fault2SrcInFault1Reg::kPinREFWBOpenInFault1Reg:
        if(configuration[static_cast<uint8_t>(device)]
                .fault2_src_cfg[cur_record]
                .usage == Fault2SrcUsage::kFault2SrcUsed){
          reg_data.bits.rfwboe_bit = 1;
        }
        break;
      case Fault2SrcInFault1Reg::kPinREFWBShortInFault1Reg:
        if(configuration[static_cast<uint8_t>(device)]
                .fault2_src_cfg[cur_record]
                .usage == Fault2SrcUsage::kFault2SrcUsed){
          reg_data.bits.rfwbse_bit = 1;
        }
        break;
    }
  }
  return reg_data.byte;
}

This is a monster of a function which can certainly be turned into a smaller functions. The amount of indirection, and number of lines for an if-statement conditional seems suspicious. It would be pretty easy to create a helper function to make that really straightforward to read.

Use of placement new

Do you really need it? It's an obscure feature, and it doesn't look like you're assigning the contents in most places to a dedicated address. What's wrong with automatic variables? I know embedded systems are memory sensitive, but usually placement new is for setting a variable at a specific memory location, or in a memory pool. If you're not doing either, why complicate the situation?

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.