2
\$\begingroup\$

I have been developing a software driver for the internal a/d converter on this platform. The a/d converter peripheral is described in following document. The driver is written in C++ and is build upon existing driver which has been developed by the platform manufacturer in C language. The reason why I have decided to wrap existing driver into the C++ is that I would like to achieve a state where this driver constitutes a consistent hardware abstraction layer with my other drivers already written in C++. During driver development I have had in my mind below given hardware schematic which describes how the internal a/d converter will be used in my application

enter image description here

As far as the driver design I have chosen following approach:

  • I have decided to encapsulate the driver into a class AdcInt which offers an interface for the a/d converter usage from the higher software layer. The intended usage of the driver is following. After instantiation of the driver the initialize method is called. The update method has to be called in periodic manner (e.g. from RTOS task) with appropriate period in respect to the sampled physical quantities. As soon as the isReady method returns true the client code can start reading a/d samples via getValue method or monitor alarm occurens via isAlarmActive method

  • in respect to the fact that the a/d peripheral can be configured in rich manner I have decided to define special class AdcIntCfgManager which manages this configuration i.e. it works upon the AdcIntCfg struct (which is filled by the higher layer programmer) and offers set of methods for retrieving the configuration data

AdcInt.h

#include "AdcIntCfgManager.h"
#include "xsysmon.h"
#include <cstdint>

class AdcInt
{
public:
  enum class Input
  {
    kChannel00,
    kChannel01,
    kChannel02,
    kChannel03,
    kChannel04,
    kChannel05,
    kChannel06,
    kChannel07,
    kChannel08,
    kChannel09,
    kChannel10,
    kChannel11,
    kChannel12,
    kNoChannels,
    kOnChipTemp,
    kVccInt,
    kVccAux,
    kVrefP,
    kVrefN,
    kVBram,
    kVccPInt,
    kVccPAux,
    kVccOddr,
    kNoInputs
  };

  AdcInt(AdcIntCfgManager *_configuration_manager, uint16_t _xadc_device_id,
         uint8_t _xadc_interrupt_id);

  void initialize();

  void update();

  bool isReady();

  bool getValue(Input input, float &var);
  
  bool isAlarmActive(AdcIntCfgManager::Alarm alarm);

  void handle_interrupt();

private:
  
  class AnalogInput
  {
  public:
    AnalogInput();

    void initialize(XSysMon *_xadc_driver, uint8_t _addr, bool _enable,
                    float _norm);

    bool isEnabled();

    float getValue();

    bool isReady();

    void notify();

  private:
    uint8_t addr;
    uint16_t value;
    float norm;
    bool enabled;
    bool ready;
    AdcIntCfgManager::ChannelType type;
    XSysMon *xadc_driver;
  };

  class Sensor
  {
  public:
    Sensor();

    void initialize(XSysMon *_xadc_driver, uint8_t _addr, bool _enable,
                    AdcIntCfgManager::OnChipSensorType _type);

    bool isEnabled();

    float getValue();

    bool isReady();

    void notify();

  private:
    uint8_t addr;
    AdcIntCfgManager::OnChipSensorType type;
    uint16_t value;
    bool enabled;
    bool ready;
    XSysMon *xadc_driver;
  };

  class Alarm
  {
  public:
    Alarm();

    void initialize(bool _enable, uint32_t _activate_mask,
                    uint32_t _deactivate_mask);

    bool isEnabled();

    bool isActive();

    void notify(uint32_t status);

  private:
    uint32_t activate_mask;
    uint32_t deactivate_mask;
    bool enabled;
    bool active;
  };

  class ChannelAddressMonitor
  {
  public:
    ChannelAddressMonitor();

    void refreshAddress();

    uint8_t getChannelAddress();

  private:
    uint8_t channel_address;
  };

  static const uint32_t NON_EXISTING_ALARM_ACTIVE_MASK = 0x00000000;
  static const uint32_t NON_EXISTING_ALARM_DEACTIVE_MASK = 0x00000000;

  static const uint64_t sensor_masks[static_cast<uint8_t>(
      AdcIntCfgManager::OnChipSensor::kNoOnChipSensors)];

  static const uint16_t calibration_masks[static_cast<uint8_t>(
      AdcIntCfgManager::Calibration::kNoCalibrations)];

  static const uint32_t
      alarm_masks[static_cast<uint8_t>(AdcIntCfgManager::Alarm::kNoAlarms)];

  static const uint16_t threshold_masks[static_cast<uint8_t>(
      AdcIntCfgManager::AlarmThresholdName::kNoAlarmThresholdNames)];

  static const uint8_t sensors_address[static_cast<uint8_t>(
      AdcIntCfgManager::OnChipSensor::kNoOnChipSensors)];

  static const uint32_t alarm_activate_masks[static_cast<uint8_t>(
      AdcIntCfgManager::Alarm::kNoAlarms)];

  static const uint32_t alarm_deactivate_masks[static_cast<uint8_t>(
      AdcIntCfgManager::Alarm::kNoAlarms)];

  static bool isEndOfSequenceInterrupt(uint32_t status);

  static bool isAlarmInterrupt(uint32_t status);

  static float temperatureRawToC(uint16_t raw_temp);

  static uint16_t temperatureCToRaw(float temp);

  static float voltageRawToV(uint16_t raw_voltage);

  static uint16_t voltageVToRaw(float voltage);

  AdcIntCfgManager *configuration_manager;
  AnalogInput analog_inputs[static_cast<uint8_t>(
      AdcIntCfgManager::Channel::kNoChannels)];
  Sensor sensors[static_cast<uint8_t>(
      AdcIntCfgManager::OnChipSensor::kNoOnChipSensors)];
  Alarm alarms[static_cast<uint8_t>(AdcIntCfgManager::Alarm::kNoAlarms)];
  ChannelAddressMonitor channel_address_monitor;

  XSysMon xadc_driver;
  uint16_t xadc_device_id;
  uint8_t xadc_interrupt_id;

  void startConversion();

  bool isBusy();

  bool getChannelValue(AdcIntCfgManager::Channel channel, float &var);

  bool getOnChipSensorValue(AdcIntCfgManager::OnChipSensor sensor, float &var);

  int configureSequencerChannels(uint64_t channels_mask);

  int configureSettlingTime(uint64_t channels_mask);

  void configureCalibration();

  void configureAlarms();

  void configureLowerAlarmThreshold(AdcIntCfgManager::Alarm alarm);

  void configureUpperAlarmThreshold(AdcIntCfgManager::Alarm alarm);

  void configureOperMode();

  void configureInterrupts();

  void initializeAnalogInputs();

  void initializeSensors();

  void initializeAlarms();

  void notifySensors();

  void notifyAlarms(uint32_t status);

  uint64_t getChannelsMask();

  uint16_t getCalibrationMask();

  uint32_t getAlarmMask();

  uint16_t getAlarmThresholdLowerMask(AdcIntCfgManager::Alarm alarm);

  uint16_t getAlarmThresholdUpperMask(AdcIntCfgManager::Alarm alarm);

  uint32_t getAlarmActivateMask(AdcIntCfgManager::Alarm alarm);

  uint32_t getAlarmDeactivateMask(AdcIntCfgManager::Alarm alarm);

  uint32_t getAlarmsInterruptEnableMask();

  uint8_t getOnChipSensorAddress(AdcIntCfgManager::OnChipSensor sensor);

};

AdcInt.cpp

#include "AdcInt.h"
#include <iostream>

using namespace std;

const uint64_t AdcInt::sensor_masks[static_cast<uint8_t>(
    AdcIntCfgManager::OnChipSensor::kNoOnChipSensors)] = {
    XSM_SEQ_CH_TEMP,    XSM_SEQ_CH_VCCINT,  XSM_SEQ_CH_VCCAUX,
    XSM_SEQ_CH_VREFP,   XSM_SEQ_CH_VREFN,   XSM_SEQ_CH_VBRAM,
    XSM_SEQ_CH_VCCPINT, XSM_SEQ_CH_VCCPAUX, XSM_SEQ_CH_VCCPDRO};

const uint16_t AdcInt::calibration_masks[static_cast<uint8_t>(
    AdcIntCfgManager::Calibration::kNoCalibrations)] = {
    XSM_CFR1_CAL_ADC_OFFSET_MASK, XSM_CFR1_CAL_ADC_GAIN_OFFSET_MASK,
    XSM_CFR1_CAL_PS_OFFSET_MASK, XSM_CFR1_CAL_PS_GAIN_OFFSET_MASK};

const uint32_t AdcInt::alarm_masks[static_cast<uint8_t>(
    AdcIntCfgManager::Alarm::kNoAlarms)] = {
    XSM_CFR_OT_MASK,          XSM_CFR_ALM_TEMP_MASK,   XSM_CFR_ALM_VCCINT_MASK,
    XSM_CFR_ALM_VCCAUX_MASK,  XSM_CFR_ALM_VBRAM_MASK,  XSM_CFR_ALM_VCCPINT_MASK,
    XSM_CFR_ALM_VCCPAUX_MASK, XSM_CFR_ALM_VCCPDRO_MASK};

const uint16_t AdcInt::threshold_masks[static_cast<uint8_t>(
    AdcIntCfgManager::AlarmThresholdName::kNoAlarmThresholdNames)] = {
    XSM_ATR_TEMP_UPPER,    XSM_ATR_VCCINT_UPPER,  XSM_ATR_VCCAUX_UPPER,
    XSM_ATR_OT_UPPER,      XSM_ATR_TEMP_LOWER,    XSM_ATR_VCCINT_LOWER,
    XSM_ATR_VCCAUX_LOWER,  XSM_ATR_OT_LOWER,      XSM_ATR_VBRAM_UPPER,
    XSM_ATR_VCCPINT_UPPER, XSM_ATR_VCCPAUX_UPPER, XSM_ATR_VCCPDRO_UPPER,
    XSM_ATR_VBRAM_LOWER,   XSM_ATR_VCCPINT_LOWER, XSM_ATR_VCCPAUX_LOWER,
    XSM_ATR_VCCPDRO_LOWER};

const uint8_t AdcInt::sensors_address[static_cast<uint8_t>(
    AdcIntCfgManager::OnChipSensor::kNoOnChipSensors)] = {
    XSM_CH_TEMP,  XSM_CH_VCCINT,   XSM_CH_VCCAUX,  XSM_CH_VREFP,  XSM_CH_VREFN,
    XSM_CH_VBRAM, XSM_CH_VCCLPINT, XSM_CH_VCCPAUX, XSM_CH_VCCPDRO};

const uint32_t AdcInt::alarm_activate_masks[static_cast<uint8_t>(
    AdcIntCfgManager::Alarm::kNoAlarms)] = {XSM_IPIXR_OT_MASK,
                                            XSM_IPIXR_TEMP_MASK,
                                            XSM_IPIXR_VCCINT_MASK,
                                            XSM_IPIXR_VCCAUX_MASK,
                                            XSM_IPIXR_VBRAM_MASK,
                                            NON_EXISTING_ALARM_ACTIVE_MASK,
                                            NON_EXISTING_ALARM_ACTIVE_MASK,
                                            NON_EXISTING_ALARM_ACTIVE_MASK};

const uint32_t AdcInt::alarm_deactivate_masks[static_cast<uint8_t>(
    AdcIntCfgManager::Alarm::kNoAlarms)] = {
    XSM_IPIXR_OT_DEACTIVE_MASK,       XSM_IPIXR_TEMP_DEACTIVE_MASK,
    NON_EXISTING_ALARM_DEACTIVE_MASK, NON_EXISTING_ALARM_DEACTIVE_MASK,
    NON_EXISTING_ALARM_DEACTIVE_MASK, NON_EXISTING_ALARM_DEACTIVE_MASK,
    NON_EXISTING_ALARM_DEACTIVE_MASK, NON_EXISTING_ALARM_DEACTIVE_MASK};

AdcInt::AdcInt(AdcIntCfgManager *_configuration_manager,
               uint16_t _xadc_device_id, uint8_t _xadc_interrupt_id)
{
  configuration_manager = _configuration_manager;
  xadc_device_id = _xadc_device_id;
  xadc_interrupt_id = _xadc_interrupt_id;
}

void AdcInt::initialize()
{
  XSysMon_Config *xadc_cfg;
  int oper_result;
  uint64_t channels_mask;

  xadc_cfg = XSysMon_LookupConfig(xadc_device_id);
  if (xadc_cfg == NULL) {
    cout << "xadc device id hasn't been found" << endl;
  } else {
    oper_result =
        XSysMon_CfgInitialize(&xadc_driver, xadc_cfg, xadc_cfg->BaseAddress);
    if (oper_result != XST_SUCCESS) {
        cout << "xadc driver initialization failed" << endl;
    } else {
      channels_mask = getChannelsMask();
      oper_result = configureSequencerChannels(channels_mask);
      if (oper_result != XST_SUCCESS) {
        cout << "xadc sequencer channels configuration failed" << endl;
      } else {
        XSysMon_SetAdcClkDivisor(&xadc_driver,
                                 configuration_manager->getClkDivRatio());
        XSysMon_SetAvg(&xadc_driver,
                       configuration_manager->getNoAveragedSamples());
        oper_result = configureSettlingTime(channels_mask);
        if (oper_result != XST_SUCCESS) {
          cout << "xadc settling time configuration failed" << endl;
        } else {
          configureCalibration();
          configureAlarms();
          configureOperMode();
          initializeAnalogInputs();
          initializeSensors();
          initializeAlarms();
          configureInterrupts();
        }
      }
    }
  }
}

void AdcInt::update()
{
  if (isBusy() == false) {
    startConversion();
  }
}

bool AdcInt::isReady()
{
  for (AnalogInput &input : analog_inputs) {
    if (input.isReady() == false) {
      return false;
    }
  }

  for (Sensor &sensor : sensors) {
    if (sensor.isReady() == false) {
      return false;
    }
  }
  return true;
}

bool AdcInt::getValue(Input input, float &var)
{
  if (static_cast<uint8_t>(input) < static_cast<uint8_t>(Input::kNoChannels)) {
    return getChannelValue(static_cast<AdcIntCfgManager::Channel>(input), var);
  } else if ((static_cast<uint8_t>(input) >
              static_cast<uint8_t>(Input::kNoChannels)) &&
             (static_cast<uint8_t>(input) <
              static_cast<uint8_t>(Input::kNoInputs))) {
    uint8_t sensor_index = static_cast<uint8_t>(input) -
                           static_cast<uint8_t>(Input::kNoChannels) - 1;
    return getOnChipSensorValue(
        static_cast<AdcIntCfgManager::OnChipSensor>(sensor_index), var);
  } else {
    return false;
  }
}

bool AdcInt::isAlarmActive(AdcIntCfgManager::Alarm alarm)
{
  return alarms[static_cast<uint8_t>(alarm)].isActive();
}

void AdcInt::handle_interrupt()
{
  uint32_t status = XSysMon_IntrGetStatus(&xadc_driver);

  if (isEndOfSequenceInterrupt(status)) {
    analog_inputs[channel_address_monitor.getChannelAddress()].notify();
    notifySensors();
    channel_address_monitor.refreshAddress();
  } else if (isAlarmInterrupt(status)) {
    notifyAlarms(status);
  }
}

AdcInt::AnalogInput::AnalogInput()
{
  value = 0;
  ready = true;
  enabled = false;
}

void AdcInt::AnalogInput::initialize(XSysMon *_xadc_driver, uint8_t _addr,
                                     bool _enable, float _norm)
{
  addr = _addr;
  enabled = _enable;
  norm = _norm;
  xadc_driver = _xadc_driver;
}

bool AdcInt::AnalogInput::isEnabled()
{
  return enabled;
}

float AdcInt::AnalogInput::getValue()
{
  return (value / 4096.0 * norm);
}

bool AdcInt::AnalogInput::isReady()
{
  return ready;
}

void AdcInt::AnalogInput::notify()
{
  if (enabled) {
    value = XSysMon_GetAdcData(xadc_driver, addr);
  }
}

AdcInt::Sensor::Sensor()
{
  value = 0;
  ready = true;
  enabled = false;
}

void AdcInt::Sensor::initialize(XSysMon *_xadc_driver, uint8_t _addr,
                                bool _enable,
                                AdcIntCfgManager::OnChipSensorType _type)
{
  addr = _addr;
  type = _type;
  enabled = _enable;
  xadc_driver = _xadc_driver;
}

bool AdcInt::Sensor::isEnabled()
{
  return enabled;
}

float AdcInt::Sensor::getValue()
{
  float ret_val;

  if (type == AdcIntCfgManager::OnChipSensorType::kVoltageSensor) {
    ret_val = voltageRawToV(value);
  } else if (type == AdcIntCfgManager::OnChipSensorType::kTemperatureSensor) {
    ret_val = temperatureRawToC(value);
  }

  return ret_val;
}

bool AdcInt::Sensor::isReady()
{
  return ready;
}

void AdcInt::Sensor::notify()
{
  if (enabled) {
    value = XSysMon_GetAdcData(xadc_driver, addr);
  }
}

AdcInt::Alarm::Alarm()
{
  active = false;
  enabled = false;
}

void AdcInt::Alarm::initialize(bool _enable, uint32_t _activate_mask,
                               uint32_t _deactivate_mask)
{
  activate_mask = _activate_mask;
  deactivate_mask = _deactivate_mask;
  enabled = _enable;
}

bool AdcInt::Alarm::isEnabled()
{
  return enabled;
}

bool AdcInt::Alarm::isActive()
{
  return active;
}

void AdcInt::Alarm::notify(uint32_t status)
{
  if (enabled) {
    if (status & activate_mask) {
      active = true;
    } else if (status & deactivate_mask) {
      active = false;
    }
  }
}

AdcInt::ChannelAddressMonitor::ChannelAddressMonitor()
{
  channel_address = 0;
}

void AdcInt::ChannelAddressMonitor::refreshAddress()
{
  if (++channel_address ==
      static_cast<uint8_t>(AdcIntCfgManager::Channel::kNoChannels)) {
    channel_address = 0;
  }
}

uint8_t AdcInt::ChannelAddressMonitor::getChannelAddress()
{
  return channel_address;
}

void AdcInt::startConversion()
{
  XSysMon_StartAdcConversion(&xadc_driver);
}

bool AdcInt::isBusy()
{
  return ((XSysMon_GetStatus(&xadc_driver) & XSM_SR_BUSY_MASK) != 0);
}

bool AdcInt::getChannelValue(AdcIntCfgManager::Channel channel, float &var)
{
  bool enabled = false;
  uint8_t index = static_cast<uint8_t>(channel);
  if (analog_inputs[index].isEnabled()) {
    var = analog_inputs[index].getValue();
    enabled = true;
  }
  return enabled;
}

bool AdcInt::getOnChipSensorValue(AdcIntCfgManager::OnChipSensor sensor,
                                  float &var)
{
  bool enabled = false;
  uint8_t index = static_cast<uint8_t>(sensor);
  if (sensors[index].isEnabled()) {
    var = sensors[index].getValue();
    enabled = true;
  }
  return enabled;
}

bool AdcInt::isEndOfSequenceInterrupt(uint32_t status)
{
  if (status & XSM_IPIXR_EOS_MASK) {
    return true;
  } else {
    return false;
  }
}

bool AdcInt::isAlarmInterrupt(uint32_t status)
{
  if ((status & XSM_IPIXR_VBRAM_MASK) |
      (status & XSM_IPIXR_TEMP_DEACTIVE_MASK) |
      (status & XSM_IPIXR_OT_DEACTIVE_MASK) | (status & XSM_IPIXR_VCCAUX_MASK) |
      (status & XSM_IPIXR_VCCINT_MASK) | (status & XSM_IPIXR_TEMP_MASK) |
      (status & XSM_IPIXR_OT_MASK)) {
    return true;
  } else {
    return false;
  }
}

float AdcInt::temperatureRawToC(uint16_t raw_temp)
{
  // see ug480 on page 33
  return ((raw_temp * 503.975) / 4096.0 - 273.15);
}

uint16_t AdcInt::temperatureCToRaw(float temp)
{
  // see ug480 on page 33
  return (4096.0 * (temp + 273.15) / 503.975);
}

float AdcInt::voltageRawToV(uint16_t raw_voltage)
{
  // see ug480 on page 34
  return (raw_voltage * 3.0 / 4096.0);
}

uint16_t AdcInt::voltageVToRaw(float voltage)
{
  // see ug480 on page 34
  return (voltage * 4096.0 / 3.0);
}

int AdcInt::configureSequencerChannels(uint64_t channels_mask)
{
  // disable sequencer before configuration (please see the xsysmon_extmux_example.c from Xilinx)
  XSysMon_SetSequencerMode(&xadc_driver, XSM_SEQ_MODE_SAFE);
  int oper_result = XSysMon_SetSeqChEnables(&xadc_driver, channels_mask);
  return oper_result;
}

int AdcInt::configureSettlingTime(uint64_t channels_mask)
{
  int oper_result = XST_SUCCESS;
  if (configuration_manager->isIncreasedSettlingTimeEnabled()) {
    oper_result = XSysMon_SetSeqAcqTime(&xadc_driver, channels_mask);
  }
  return oper_result;
}

void AdcInt::configureCalibration()
{
  uint16_t calibration_mask = getCalibrationMask();
  XSysMon_SetCalibEnables(&xadc_driver, calibration_mask);
}

void AdcInt::configureAlarms()
{
  uint32_t alarms_mask = getAlarmMask();
  XSysMon_SetAlarmEnables(&xadc_driver, alarms_mask);

  for (uint8_t alarm = 0;
       alarm < static_cast<uint8_t>(AdcIntCfgManager::Alarm::kNoAlarms);
       alarm++) {
    if (configuration_manager->isAlarmEnabled(
            static_cast<AdcIntCfgManager::Alarm>(alarm))) {
      configureLowerAlarmThreshold(static_cast<AdcIntCfgManager::Alarm>(alarm));
      configureUpperAlarmThreshold(static_cast<AdcIntCfgManager::Alarm>(alarm));
    }
  }
}

void AdcInt::configureLowerAlarmThreshold(AdcIntCfgManager::Alarm alarm)
{
  float threshold;
  uint8_t threshold_mask;
  uint16_t raw_threshold;

  threshold = configuration_manager->getAlarmThresholdLower(alarm);
  threshold_mask = getAlarmThresholdLowerMask(alarm);
  if (configuration_manager->getAlarmType(alarm) ==
      AdcIntCfgManager::AlarmType::kTemperatureAlarm) {
    raw_threshold = temperatureCToRaw(threshold);
  } else if (configuration_manager->getAlarmType(alarm) ==
             AdcIntCfgManager::AlarmType::kVoltageAlarm) {
    raw_threshold = voltageVToRaw(threshold);
  }
  XSysMon_SetAlarmThreshold(&xadc_driver, threshold_mask, raw_threshold);
}

void AdcInt::configureUpperAlarmThreshold(AdcIntCfgManager::Alarm alarm)
{
  float threshold;
  uint8_t threshold_mask;
  uint16_t raw_threshold;

  threshold = configuration_manager->getAlarmThresholdUpper(alarm);
  threshold_mask = getAlarmThresholdUpperMask(alarm);
  if (configuration_manager->getAlarmType(alarm) ==
      AdcIntCfgManager::AlarmType::kTemperatureAlarm) {
    raw_threshold = temperatureCToRaw(threshold);
  } else if (configuration_manager->getAlarmType(alarm) ==
             AdcIntCfgManager::AlarmType::kVoltageAlarm) {
    raw_threshold = voltageVToRaw(threshold);
  }
  XSysMon_SetAlarmThreshold(&xadc_driver, threshold_mask, raw_threshold);
}

void AdcInt::configureOperMode()
{
  // external multiplexer connection
  XSysMon_SetExtenalMux(&xadc_driver, XSM_CH_VPVN);
  // single pass mode of operation
  XSysMon_SetSequencerMode(&xadc_driver, XSM_SEQ_MODE_ONEPASS);
}

void AdcInt::configureInterrupts()
{
  XSysMon_IntrEnable(&xadc_driver,
                     XSM_IPIXR_EOS_MASK | getAlarmsInterruptEnableMask());
  XSysMon_IntrGlobalEnable(&xadc_driver);
  platform::drivers::InterruptController::instance().connect(xadc_interrupt_id,
                                                             *this);
}

void AdcInt::initializeAnalogInputs()
{
  for (uint8_t input = 0;
       input < static_cast<uint8_t>(AdcIntCfgManager::Channel::kNoChannels);
       input++) {
    analog_inputs[input].initialize(
        &xadc_driver, XSM_CH_VPVN,
        configuration_manager->isChannelEnabled(
            static_cast<AdcIntCfgManager::Channel>(input)),
        configuration_manager->getChannelNorm(
            static_cast<AdcIntCfgManager::Channel>(input)));
  }
}

void AdcInt::initializeSensors()
{
  for (uint8_t sensor = 0;
       sensor <
       static_cast<uint8_t>(AdcIntCfgManager::OnChipSensor::kNoOnChipSensors);
       sensor++) {
    sensors[sensor].initialize(
        &xadc_driver,
        getOnChipSensorAddress(
            static_cast<AdcIntCfgManager::OnChipSensor>(sensor)),
        configuration_manager->isOnChipSensorEnabled(
            static_cast<AdcIntCfgManager::OnChipSensor>(sensor)),
        configuration_manager->getSensorType(
            static_cast<AdcIntCfgManager::OnChipSensor>(sensor)));
  }
}

void AdcInt::initializeAlarms()
{
  for (uint8_t alarm = 0;
       alarm < static_cast<uint8_t>(AdcIntCfgManager::Alarm::kNoAlarms);
       alarm++) {
    alarms[alarm].initialize(
        configuration_manager->isAlarmEnabled(
            static_cast<AdcIntCfgManager::Alarm>(alarm)),
        getAlarmActivateMask(static_cast<AdcIntCfgManager::Alarm>(alarm)),
        getAlarmDeactivateMask(static_cast<AdcIntCfgManager::Alarm>(alarm)));
  }
}

void AdcInt::notifySensors()
{
  for (Sensor &sensor : sensors) {
    sensor.notify();
  }
}

void AdcInt::notifyAlarms(uint32_t status)
{
  for (Alarm &alarm : alarms) {
    alarm.notify(status);
  }
}

uint64_t AdcInt::getChannelsMask()
{
  uint64_t channels_mask = 0;

  if (configuration_manager->isCalibrationEnabled()) {
    channels_mask |= XSM_SEQ_CH_CALIB;
  }

  for (uint8_t sensor = 0;
       sensor <
       static_cast<uint8_t>(AdcIntCfgManager::OnChipSensor::kNoOnChipSensors);
       sensor++) {
    if (configuration_manager->isOnChipSensorEnabled(
            static_cast<AdcIntCfgManager::OnChipSensor>(sensor))) {
      channels_mask |= sensor_masks[sensor];
    }
  }

  // external multiplexer connection
  channels_mask |= XSM_SEQ_CH_VPVN;

  return channels_mask;
}

uint16_t AdcInt::getCalibrationMask()
{
  uint16_t calibration_mask = 0;

  for (uint8_t calibration = 0;
       calibration <
       static_cast<uint8_t>(AdcIntCfgManager::Calibration::kNoCalibrations);
       calibration++) {
    if (configuration_manager->isCalibrationTypeEnabled(
            static_cast<AdcIntCfgManager::Calibration>(calibration))) {
      calibration_mask |= calibration_masks[calibration];
    }
  }

  return calibration_mask;
}

uint32_t AdcInt::getAlarmMask()
{
  uint32_t alarm_mask = 0;

  for (uint8_t alarm = 0;
       alarm < static_cast<uint8_t>(AdcIntCfgManager::Alarm::kNoAlarms);
       alarm++) {
    if (configuration_manager->isAlarmEnabled(
            static_cast<AdcIntCfgManager::Alarm>(alarm))) {
      alarm_mask |= alarm_masks[alarm];
    }
  }

  return alarm_mask;
}

uint16_t AdcInt::getAlarmThresholdLowerMask(AdcIntCfgManager::Alarm alarm)
{
  return (threshold_masks[static_cast<uint8_t>(
      configuration_manager->getAlarmLowerThresholdName(alarm))]);
}

uint16_t AdcInt::getAlarmThresholdUpperMask(AdcIntCfgManager::Alarm alarm)
{
  return (threshold_masks[static_cast<uint8_t>(
      configuration_manager->getAlarmUpperThresholdName(alarm))]);
}

uint32_t AdcInt::getAlarmActivateMask(AdcIntCfgManager::Alarm alarm)
{
  return alarm_activate_masks[static_cast<uint8_t>(alarm)];
}

uint32_t AdcInt::getAlarmDeactivateMask(AdcIntCfgManager::Alarm alarm)
{
  return alarm_deactivate_masks[static_cast<uint8_t>(alarm)];
}

uint32_t AdcInt::getAlarmsInterruptEnableMask()
{
  uint32_t alarms_interrupt_enable_mask = 0;
  for (uint8_t alarm = 0;
       alarm < static_cast<uint8_t>(AdcIntCfgManager::Alarm::kNoAlarms);
       alarm++) {
    if (configuration_manager->isAlarmEnabled(
            static_cast<AdcIntCfgManager::Alarm>(alarm))) {
      alarms_interrupt_enable_mask |= alarm_activate_masks[alarm];
    }
  }
  return alarms_interrupt_enable_mask;
}

uint8_t AdcInt::getOnChipSensorAddress(AdcIntCfgManager::OnChipSensor sensor)
{
  return sensors_address[static_cast<uint8_t>(sensor)];
}

AdcIntCfgManager.h

#include <cstdint>

class AdcIntCfgManager
{
public:
  enum class Channel {
    kChannel00,
    kChannel01,
    kChannel02,
    kChannel03,
    kChannel04,
    kChannel05,
    kChannel06,
    kChannel07,
    kChannel08,
    kChannel09,
    kChannel10,
    kChannel11,
    kChannel12,
    kNoChannels
  };

  enum class OnChipSensor {
    kOnChipTemp,
    kVccInt,
    kVccAux,
    kVrefP,
    kVrefN,
    kVBram,
    kVccPInt,
    kVccPAux,
    kVccOddr,
    kNoOnChipSensors
  };

  enum class ChannelType { kUnipolar, kBipolar };

  enum class ChannelEnable { kChannelDisabled, kChannelEnabled };

  struct ChannelCfg
  {
    Channel channel;
    ChannelType type;
    ChannelEnable enable;
    float norm;
  };

  enum class AdcClockDivRatio {
    kADCCLK_DCLK_DIV_002, 
    kADCCLK_DCLK_DIV_003 = 3, 
    kADCCLK_DCLK_DIV_004 = 4, 
    kADCCLK_DCLK_DIV_255 = 255 
  };

  enum class OnChipSensorType { kVoltageSensor, kTemperatureSensor };

  enum class OnChipSensorEnable { kSensorDisabled, kSensorEnabled };

  struct OnChipSensorsCfg
  {
    OnChipSensor sensor;
    OnChipSensorType type;
    OnChipSensorEnable enable;
  };

  enum class Averaging {
    kAveragingFrom_0_Samples,
    kAveragingFrom_16_Samples,
    kAveragingFrom_64_Samples,
    kAveragingFrom_256_Samples
  };

  enum class Calibration {
    kAdcOffsetCorrection,
    kAdcOffsetGainCorrection,
    kSupplySensorOffsetCorrection,
    kSupplySensorOffsetGainCorrection,
    kNoCalibrations
  };

  enum class CalibrationEnable { kCalibrationDisabled, kCalibrationEnabled };

  struct CalibrationCfg
  {
    Calibration calibration;
    CalibrationEnable enable;
  };

  enum class SettlingTime { kSettlingTime_4_ADCCLK, kSettlingTime_10_ADCCLK };

  enum class Alarm {
    kOverTemperature,
    kOnChipTemperature,
    kVccInt,
    kVccAux,
    kVBram,
    kVccPInt,
    kVccPAux,
    kVccOddr,
    kNoAlarms
  };

  enum class AlarmType { kTemperatureAlarm, kVoltageAlarm };

  enum class AlarmEnable { kAlarmDisabled, kAlarmEnabled };

  enum class AlarmThresholdName {
    kOnChipTemperatureUpper,
    kVccIntUpper,
    kVccAuxUpper,
    kOverTemperatureSet,
    kOnChipTemperatureLower,
    kVccIntLower,
    kVccAuxLower,
    kOverTemperatureReset,
    kVBramUpper,
    kVccPIntUpper,
    kVccPAuxUpper,
    kVccOddrUpper,
    kVBramLower,
    kVccPIntLower,
    kVccPAuxLower,
    kVccOddrLower,
    kNoAlarmThresholdNames
  };

  struct AlarmThreshold
  {
    AlarmThresholdName name;
    float value;
  };

  struct AlarmsCfg
  {
    Alarm alarm;
    AlarmType type;
    AlarmEnable enable;
    AlarmThreshold lower_threshold;
    AlarmThreshold upper_threshold;
  };

  struct AdcIntCfg
  {
    ChannelCfg channels_cfg[static_cast<uint8_t>(
        Channel::
            kNoChannels)]; 
    OnChipSensorsCfg sensors_cfg[static_cast<uint8_t>(
        OnChipSensor::kNoOnChipSensors)]; 
    AdcClockDivRatio
        clock_div_ratio; 
    SettlingTime settling_time; 
    Averaging averaging; 
    CalibrationCfg calibration_cfg[static_cast<uint8_t>(
        Calibration::kNoCalibrations)]; 
    AlarmsCfg alarms_cfg[static_cast<uint8_t>(
        Alarm::
            kNoAlarms)]; 
  };

  AdcIntCfgManager(const AdcIntCfg *_configuration);

  uint8_t getClkDivRatio();

  bool isIncreasedSettlingTimeEnabled();

  uint8_t getNoAveragedSamples();

  bool isCalibrationEnabled();

  bool isCalibrationTypeEnabled(Calibration type);

  bool isAlarmEnabled(Alarm alarm);

  float getAlarmThresholdLower(Alarm alarm);

  float getAlarmThresholdUpper(Alarm alarm);

  AlarmType getAlarmType(Alarm alarm);

  AlarmThresholdName getAlarmLowerThresholdName(Alarm alarm);

  AlarmThresholdName getAlarmUpperThresholdName(Alarm alarm);

  bool isChannelEnabled(Channel channel);

  ChannelType getChannelType(Channel channel);

  float getChannelNorm(Channel channel);

  bool isOnChipSensorEnabled(OnChipSensor sensor);

  OnChipSensorType getSensorType(OnChipSensor sensor);

private:

  const AdcIntCfg *configuration;

};

AdcIntCfgManager.cpp

#include "AdcIntCfgManager.h"

AdcIntCfgManager::AdcIntCfgManager(const AdcIntCfg *_configuration)
{
  configuration = _configuration;
}

uint8_t AdcIntCfgManager::getClkDivRatio()
{
  return static_cast<uint8_t>(configuration->clock_div_ratio);
}

bool AdcIntCfgManager::isIncreasedSettlingTimeEnabled()
{
  return (configuration->settling_time ==
          SettlingTime::kSettlingTime_10_ADCCLK);
}

uint8_t AdcIntCfgManager::getNoAveragedSamples()
{
  return static_cast<uint8_t>(configuration->averaging);
}

bool AdcIntCfgManager::isCalibrationEnabled()
{
  bool ret_val = false;

  for (uint8_t i = 0; i < static_cast<uint8_t>(Calibration::kNoCalibrations);
       i++) {
    if (configuration->calibration_cfg[i].enable ==
        CalibrationEnable::kCalibrationEnabled) {
      ret_val = true;
      break;
    }
  }
  return ret_val;
}

bool AdcIntCfgManager::isCalibrationTypeEnabled(Calibration type)
{
  if (configuration->calibration_cfg[static_cast<uint8_t>(type)].enable ==
      CalibrationEnable::kCalibrationEnabled) {
    return true;
  } else {
    return false;
  }
}

bool AdcIntCfgManager::isAlarmEnabled(Alarm alarm)
{
  return (configuration->alarms_cfg[static_cast<uint8_t>(alarm)].enable ==
          AlarmEnable::kAlarmEnabled);
}

float AdcIntCfgManager::getAlarmThresholdLower(Alarm alarm)
{
  return (configuration->alarms_cfg[static_cast<uint8_t>(alarm)]
              .lower_threshold.value);
}

float AdcIntCfgManager::getAlarmThresholdUpper(Alarm alarm)
{
  return (configuration->alarms_cfg[static_cast<uint8_t>(alarm)]
              .upper_threshold.value);
}

AdcIntCfgManager::AlarmType AdcIntCfgManager::getAlarmType(Alarm alarm)
{
  return (configuration->alarms_cfg[static_cast<uint8_t>(alarm)].type);
}

AdcIntCfgManager::AlarmThresholdName
AdcIntCfgManager::getAlarmLowerThresholdName(Alarm alarm)
{
  return (configuration->alarms_cfg[static_cast<uint8_t>(alarm)]
              .lower_threshold.name);
}

AdcIntCfgManager::AlarmThresholdName
AdcIntCfgManager::getAlarmUpperThresholdName(Alarm alarm)
{
  return (configuration->alarms_cfg[static_cast<uint8_t>(alarm)]
              .upper_threshold.name);
}

bool AdcIntCfgManager::isChannelEnabled(Channel channel)
{
  if (configuration->channels_cfg[static_cast<uint8_t>(channel)].enable ==
      ChannelEnable::kChannelEnabled) {
    return true;
  } else {
    return false;
  }
}

float AdcIntCfgManager::getChannelNorm(Channel channel)
{
  return (configuration->channels_cfg[static_cast<uint8_t>(channel)].norm);
}

AdcIntCfgManager::ChannelType AdcIntCfgManager::getChannelType(Channel channel)
{
  return (configuration->channels_cfg[static_cast<uint8_t>(channel)].type);
}

bool AdcIntCfgManager::isOnChipSensorEnabled(OnChipSensor sensor)
{
  if (configuration->sensors_cfg[static_cast<uint8_t>(sensor)].enable ==
      OnChipSensorEnable::kSensorEnabled) {
    return true;
  } else {
    return false;
  }
}

AdcIntCfgManager::OnChipSensorType
AdcIntCfgManager::getSensorType(OnChipSensor sensor)
{
  return (configuration->sensors_cfg[static_cast<uint8_t>(sensor)].type);
}

The intended usage of the driver is following:

  • whole the configuration information are in the FpgaPeripheralsCfg class
  • the instance of the AdcInt is along with instances of other drivers in the "container" FpgaDrivers

FpgaPeripheralsCfg.h

#include "AdcIntCfgManager.h"

class FpgaPeripheralsCfg
{
public:
  AdcIntCfgManager adc_int_cfg_manager;
  
  FpgaPeripheralsCfg() :
    adc_int_cfg_manager(&adc_int_cfg)
  {
  }

private:
  AdcIntCfgManager::AdcIntCfg adc_int_cfg = {
      {{AdcIntCfgManager::Channel::kChannel00,
        AdcIntCfgManager::ChannelType::kBipolar,
        AdcIntCfgManager::ChannelEnable::kChannelEnabled, 100.0},
       {AdcIntCfgManager::Channel::kChannel01,
        AdcIntCfgManager::ChannelType::kBipolar,
        AdcIntCfgManager::ChannelEnable::kChannelEnabled, 100.0},
       {AdcIntCfgManager::Channel::kChannel02,
        AdcIntCfgManager::ChannelType::kBipolar,
        AdcIntCfgManager::ChannelEnable::kChannelEnabled, 100.0},
       {AdcIntCfgManager::Channel::kChannel03,
        AdcIntCfgManager::ChannelType::kBipolar,
        AdcIntCfgManager::ChannelEnable::kChannelEnabled, 100.0},
       {AdcIntCfgManager::Channel::kChannel04,
        AdcIntCfgManager::ChannelType::kBipolar,
        AdcIntCfgManager::ChannelEnable::kChannelEnabled, 100.0},
       {AdcIntCfgManager::Channel::kChannel05,
        AdcIntCfgManager::ChannelType::kBipolar,
        AdcIntCfgManager::ChannelEnable::kChannelEnabled, 100.0},
       {AdcIntCfgManager::Channel::kChannel06,
        AdcIntCfgManager::ChannelType::kBipolar,
        AdcIntCfgManager::ChannelEnable::kChannelEnabled, 100.0},
       {AdcIntCfgManager::Channel::kChannel07,
        AdcIntCfgManager::ChannelType::kBipolar,
        AdcIntCfgManager::ChannelEnable::kChannelEnabled, 100.0},
       {AdcIntCfgManager::Channel::kChannel08,
        AdcIntCfgManager::ChannelType::kBipolar,
        AdcIntCfgManager::ChannelEnable::kChannelEnabled, 100.0},
       {AdcIntCfgManager::Channel::kChannel09,
        AdcIntCfgManager::ChannelType::kBipolar,
        AdcIntCfgManager::ChannelEnable::kChannelEnabled, 100.0},
       {AdcIntCfgManager::Channel::kChannel10,
        AdcIntCfgManager::ChannelType::kBipolar,
        AdcIntCfgManager::ChannelEnable::kChannelEnabled, 100.0},
       {AdcIntCfgManager::Channel::kChannel11,
        AdcIntCfgManager::ChannelType::kBipolar,
        AdcIntCfgManager::ChannelEnable::kChannelEnabled, 100.0},
       {AdcIntCfgManager::Channel::kChannel12,
        AdcIntCfgManager::ChannelType::kUnipolar,
        AdcIntCfgManager::ChannelEnable::kChannelEnabled, 3.0}},
      {{AdcIntCfgManager::OnChipSensor::kOnChipTemp,
        AdcIntCfgManager::OnChipSensorType::kTemperatureSensor,
        AdcIntCfgManager::OnChipSensorEnable::kSensorEnabled},
       {AdcIntCfgManager::OnChipSensor::kVccInt,
        AdcIntCfgManager::OnChipSensorType::kVoltageSensor,
        AdcIntCfgManager::OnChipSensorEnable::kSensorEnabled},
       {AdcIntCfgManager::OnChipSensor::kVccAux,
        AdcIntCfgManager::OnChipSensorType::kVoltageSensor,
        AdcIntCfgManager::OnChipSensorEnable::kSensorEnabled},
       {AdcIntCfgManager::OnChipSensor::kVrefP,
        AdcIntCfgManager::OnChipSensorType::kVoltageSensor,
        AdcIntCfgManager::OnChipSensorEnable::kSensorEnabled},
       {AdcIntCfgManager::OnChipSensor::kVrefN,
        AdcIntCfgManager::OnChipSensorType::kVoltageSensor,
        AdcIntCfgManager::OnChipSensorEnable::kSensorEnabled},
       {AdcIntCfgManager::OnChipSensor::kVBram,
        AdcIntCfgManager::OnChipSensorType::kVoltageSensor,
        AdcIntCfgManager::OnChipSensorEnable::kSensorEnabled},
       {AdcIntCfgManager::OnChipSensor::kVccPInt,
        AdcIntCfgManager::OnChipSensorType::kVoltageSensor,
        AdcIntCfgManager::OnChipSensorEnable::kSensorEnabled},
       {AdcIntCfgManager::OnChipSensor::kVccPAux,
        AdcIntCfgManager::OnChipSensorType::kVoltageSensor,
        AdcIntCfgManager::OnChipSensorEnable::kSensorEnabled},
       {AdcIntCfgManager::OnChipSensor::kVccOddr,
        AdcIntCfgManager::OnChipSensorType::kVoltageSensor,
        AdcIntCfgManager::OnChipSensorEnable::kSensorEnabled}},
      .clock_div_ratio =
          AdcIntCfgManager::AdcClockDivRatio::kADCCLK_DCLK_DIV_002,
      .settling_time = AdcIntCfgManager::SettlingTime::kSettlingTime_4_ADCCLK,
      .averaging = AdcIntCfgManager::Averaging::kAveragingFrom_16_Samples,
      {{AdcIntCfgManager::Calibration::kAdcOffsetCorrection,
        AdcIntCfgManager::CalibrationEnable::kCalibrationEnabled},
       {AdcIntCfgManager::Calibration::kAdcOffsetGainCorrection,
        AdcIntCfgManager::CalibrationEnable::kCalibrationEnabled},
       {AdcIntCfgManager::Calibration::kSupplySensorOffsetCorrection,
        AdcIntCfgManager::CalibrationEnable::kCalibrationEnabled},
       {AdcIntCfgManager::Calibration::kSupplySensorOffsetGainCorrection,
        AdcIntCfgManager::CalibrationEnable::kCalibrationEnabled}},
      {{AdcIntCfgManager::Alarm::kOverTemperature,
        AdcIntCfgManager::AlarmType::kTemperatureAlarm,
        AdcIntCfgManager::AlarmEnable::kAlarmEnabled,
        {AdcIntCfgManager::AlarmThresholdName::kOverTemperatureReset, 90.0},
        {AdcIntCfgManager::AlarmThresholdName::kOverTemperatureSet, 125.0}},
       {AdcIntCfgManager::Alarm::kOnChipTemperature,
        AdcIntCfgManager::AlarmType::kTemperatureAlarm,
        AdcIntCfgManager::AlarmEnable::kAlarmEnabled,
        {AdcIntCfgManager::AlarmThresholdName::kOnChipTemperatureLower, 100.0},
        {AdcIntCfgManager::AlarmThresholdName::kOnChipTemperatureUpper, 125.0}},
       {AdcIntCfgManager::Alarm::kVccInt,
        AdcIntCfgManager::AlarmType::kVoltageAlarm,
        AdcIntCfgManager::AlarmEnable::kAlarmEnabled,
        {AdcIntCfgManager::AlarmThresholdName::kVccIntLower, 2.7},
        {AdcIntCfgManager::AlarmThresholdName::kVccIntUpper, 2.9}},
       {AdcIntCfgManager::Alarm::kVccAux,
        AdcIntCfgManager::AlarmType::kVoltageAlarm,
        AdcIntCfgManager::AlarmEnable::kAlarmEnabled,
        {AdcIntCfgManager::AlarmThresholdName::kVccAuxLower, 2.7},
        {AdcIntCfgManager::AlarmThresholdName::kVccAuxUpper, 2.9}},
       {AdcIntCfgManager::Alarm::kVBram,
        AdcIntCfgManager::AlarmType::kVoltageAlarm,
        AdcIntCfgManager::AlarmEnable::kAlarmEnabled,
        {AdcIntCfgManager::AlarmThresholdName::kVBramLower, 2.7},
        {AdcIntCfgManager::AlarmThresholdName::kVBramUpper, 2.9}},
       {AdcIntCfgManager::Alarm::kVccPInt,
        AdcIntCfgManager::AlarmType::kVoltageAlarm,
        AdcIntCfgManager::AlarmEnable::kAlarmEnabled,
        {AdcIntCfgManager::AlarmThresholdName::kVccPIntLower, 2.7},
        {AdcIntCfgManager::AlarmThresholdName::kVccPIntUpper, 2.9}},
       {AdcIntCfgManager::Alarm::kVccPAux,
        AdcIntCfgManager::AlarmType::kVoltageAlarm,
        AdcIntCfgManager::AlarmEnable::kAlarmEnabled,
        {AdcIntCfgManager::AlarmThresholdName::kVccPAuxLower, 2.7},
        {AdcIntCfgManager::AlarmThresholdName::kVccPAuxUpper, 2.9}},
       {AdcIntCfgManager::Alarm::kVccOddr,
        AdcIntCfgManager::AlarmType::kVoltageAlarm,
        AdcIntCfgManager::AlarmEnable::kAlarmEnabled,
        {AdcIntCfgManager::AlarmThresholdName::kVccOddrLower, 2.7},
        {AdcIntCfgManager::AlarmThresholdName::kVccOddrUpper, 2.9}}}};  
};

FpgaDrivers.h

#include "AdcInt.h"

class FpgaDrivers
{
public:
  AdcInt adc_int;
  
  FpgaDrivers(FpgaPeripheralsCfg &cfg);

  void initialize(void);

private:
};

FpgaDrivers.cpp

#include "FpgaDrivers.h"

FpgaDrivers::FpgaDrivers(FpgaPeripheralsCfg &cfg) :
  adc_int(&(cfg.adc_int_cfg_manager),  0, 62)
{
}

void FpgaDrivers::initialize(void)
{
  adc_int.initialize();
}

main

int main(int argc, char** argv) {
        
    float adc_value;
    
    FpgaPeripheralsCfg fpga_peripherals_cfg;
    FpgaDrivers drivers(fpga_peripherals_cfg);
    drivers.initialize();
    
    drivers.adc_int.update();

    if(drivers.adc_int.isReady()) {
       drivers.adc_int.getValue(platform::drivers::fpga::AdcInt::Input::kChannel00, adc_value);
    }
            
    return 0;
}

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.

Improve this question
\$\endgroup\$
4
  • \$\begingroup\$ It's not clear which file is which because the #include file names don't match the names in the question. If you could edit the file names in the question to be the full, correct names of each file, it would make the question easier to review. \$\endgroup\$
    – Edward
    Jan 18, 2021 at 18:04
  • \$\begingroup\$ I am sorry for that. This discrepancy is caused by my naming convention for the .h and .cpp modules. I have just attempted to put it into accordance. \$\endgroup\$
    – L3sek
    Jan 19, 2021 at 15:09
  • 1
    \$\begingroup\$ Still unclear. It appears you have lumped the .h and .cpp files together under each heading. Better would be to label each file with its actual name. \$\endgroup\$
    – Edward
    Jan 19, 2021 at 15:15
  • \$\begingroup\$ I have just edited my question. \$\endgroup\$
    – L3sek
    Jan 19, 2021 at 15:25

1 Answer 1

Reset to default
3
\$\begingroup\$

const

You need more of it, in several places. Methods like these:

bool isReady();
bool getValue(Input input, float &var);
bool isAlarmActive(AdcIntCfgManager::Alarm alarm);
bool isEnabled();
float getValue();
bool isReady();

should all get a const suffix of the form

bool isReady() const;

as a promise that the method does not modify this. If any of the above do modify this, then that means the method needs to be redesigned or at least renamed.

Members like this:

AdcIntCfgManager::ChannelType type;
XSysMon *xadc_driver;

should probably be

const AdcIntCfgManager::ChannelType type;
XSysMon *const xadc_driver;

to indicate a guarantee that they won't change over the instance's lifetime even if other members do.

Boolean redundancy

if (isBusy() == false) {

should be

if (!isBusy()) {

and

  if (status & XSM_IPIXR_EOS_MASK) {
    return true;
  } else {
    return false;
  }

should be

return status & XSM_IPIXR_EOS_MASK != 0;

Redundant parens

return (value / 4096.0 * norm);

does not need outer parens.

Combined mask comparison

  if ((status & XSM_IPIXR_VBRAM_MASK) |
      (status & XSM_IPIXR_TEMP_DEACTIVE_MASK) |
      (status & XSM_IPIXR_OT_DEACTIVE_MASK) | (status & XSM_IPIXR_VCCAUX_MASK) |
      (status & XSM_IPIXR_VCCINT_MASK) | (status & XSM_IPIXR_TEMP_MASK) |
      (status & XSM_IPIXR_OT_MASK)) {

can be simplified by storing a combined mask constant somewhere else:

const int INTERRUPT_MASK =
      XSM_IPIXR_VBRAM_MASK
    | XSM_IPIXR_TEMP_DEACTIVE_MASK
    | XSM_IPIXR_OT_DEACTIVE_MASK
    | XSM_IPIXR_VCCAUX_MASK
    | XSM_IPIXR_VCCINT_MASK
    | XSM_IPIXR_TEMP_MASK
    | XSM_IPIXR_OT_MASK;
// ...
return status & INTERRUPT_MASK != 0;

Early return

Rather than

  int oper_result = XST_SUCCESS;
  if (configuration_manager->isIncreasedSettlingTimeEnabled()) {
    oper_result = XSysMon_SetSeqAcqTime(&xadc_driver, channels_mask);
  }
  return oper_result;

I usually prefer the style

    if (configuration_manager->isIncreasedSettlingTimeEnabled()) {
        return XSysMon_SetSeqAcqTime(&xadc_driver, channels_mask);
    }
    return XST_SUCCESS;

Similarly,

bool AdcIntCfgManager::isCalibrationEnabled()
{
  for (uint8_t i = 0; i < static_cast<uint8_t>(Calibration::kNoCalibrations);
       i++) {
    if (configuration->calibration_cfg[i].enable ==
        CalibrationEnable::kCalibrationEnabled) {
      return true;
    }
  }
  return false;
}

Void arguments

Whereas (void) is the preferred way of indicating no-argument functions in C, in C++ this is redundant and can simply be () in lines like

void initialize(void);

Indentation

Consider moving to a more common four-space indentation instead of two.

Initialization

Having an initialize() method separated from your constructor is somewhat of an anti-pattern in C++. The implication is that between the time that your class is constructed and before it is initialized, it will be in an invalid state. Attempt to eliminate this by doing the work of initialize() in the constructor itself. Since none of your initialization routines seem able to detect failure, this maps easily to a constructor. If you had to indicate failure, either use exceptions - which would guarantee the non-existence of an invalid instance - or less ideally, set a failure flag.

Improve this answer
\$\endgroup\$
2
  • \$\begingroup\$ Thank you very much for your comments. They are very helpful for me but it could be said that they are more or less of cosmetic. I am more interested in your opinion regarding the architecture of the driver. May I ask you for your opinion in this aspect? \$\endgroup\$
    – L3sek
    Jan 17, 2021 at 19:55
  • 1
    \$\begingroup\$ Proper use of const is not cosmetic and will have a direct impact on how much you can be assured of program correctness. Otherwise, one more thing in my recent edit. \$\endgroup\$
    – Reinderien
    Jan 17, 2021 at 20:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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