Use of enum i.s.o. uint8_t types in function calls.

[HenkHoldijk]

Not really an issue, but consider changing some of the uint8_t function parameter types into an enum.

Currently one has to pass a number for some of the function calls and runtime-checking of the value is done. This can be replaced by passing an enum and do the check compile-time (which is really what you want). I personally would enforce the enum values as you rely on them on a lower (within the function) level.

Like for the setAverage function :

enum ina226_set_average_enum {
    INA226_NO_AVERAGE = 0,
    INA226_AVERAGE_4_SAMPLES = 1,
    INA226_AVERAGE_16_SAMPLES = 2,
    INA226_AVERAGE_64_SAMPLES = 3,
    INA226_AVERAGE_128_SAMPLES = 4,
    INA226_AVERAGE_256_SAMPLES = 5,
    INA226_AVERAGE_512_SAMPLES = 6,
    INA226_AVERAGE_1024_SAMPLES = 7
  };

bool INA226::setAverage(ina226_set_average_enum avg)
{
  uint16_t mask = _readRegister(INA226_CONFIGURATION);
  mask &= ~INA226_CONF_AVERAGE_MASK;
  mask |= (avg << 9);
  _writeRegister(INA226_CONFIGURATION, mask);
  return true;
}

or even (as the checking is now done compile-time) :

void INA226::setAverage(ina226_set_average_enum avg)
{
  uint16_t mask = _readRegister(INA226_CONFIGURATION);
  mask &= ~INA226_CONF_AVERAGE_MASK;
  mask |= (avg << 9);
  _writeRegister(INA226_CONFIGURATION, mask);
  return;
}

In your application, the call would be something like :

ina226.setAverage(INA226_AVERAGE_64_SAMPLES); // Rather clear what will be done, just by reading this line of code

i.s.o.

ina226.setAverage(3); // Let's look up what 3 results into

(updated syntax highlighting)

[RobTillaart]

Thanks for the issue, Makes perfect sense and I will pick it up later as my agenda is filling up with several other tasks.

[RobTillaart]

@HenkHoldijk I have released version 0.5.2 which includes many of your proposals without breaking the interface.

See - https://github.com/RobTillaart/INA226/pull/36 - for original PR.

[RobTillaart]

Too busy with different libraries, sorry.

It is easy (without breaking code) to add

enum ina226_set_average_enum {
    INA226_1_SAMPLE = 0,
    INA226_4_SAMPLES = 1,
    INA226_16_SAMPLES = 2,
    INA226_64_SAMPLES = 3,
    INA226_128_SAMPLES = 4,
    INA226_256_SAMPLES = 5,
    INA226_512_SAMPLES = 6,
    INA226_1024_SAMPLES = 7
  }

so people can use the descriptive names you propose. I propose to leave out the AVERAGE as it is still descriptive without..

ina226.setAverage(INA226_64_SAMPLES);

[HenkHoldijk]

Rob, you can have a look at my fork https://github.com/HenkHoldijk/RobTillaart_HenkHoldijk_INA226. I updated only INA226.cpp and INA226.h

Lot of changes, including the programming of the alert pin. Running this lib now for 2 months.

[RobTillaart]

A similar enum will be added for BVCT SVCT timing

enum ina226_timing {
    INA226_1_SAMPLE = 0,
    INA226_4_SAMPLES = 1,
    INA226_16_SAMPLES = 2,
    INA226_64_SAMPLES = 3,
    INA226_128_SAMPLES = 4,
    INA226_256_SAMPLES = 5,
    INA226_512_SAMPLES = 6,
    INA226_1024_SAMPLES = 7
  }

[RobTillaart]

Rob, you can have a look at my fork https://github.com/HenkHoldijk/RobTillaart_HenkHoldijk_INA226. I updated only INA226.cpp and INA226.h

Lot of changes, including the programming of the alert pin. Running this lib now for 2 months.

Will do that when time permits Good to hear it is well tested!

[HenkHoldijk]

I'm using pieces of code like:

if (! ina226.setAlertPinPolarity(INA226_ACTIVE_HIGH))
  Serial.println("INA226 : Not able to setAlertPinPolarity");
if (! ina226.setAlertLatch(INA226_LATCH_TRANSPARENT))
  Serial.println("INA226 : Not able to setAlertLatchEnable");
if (! ina226.setAlert(INA226_SHUNT_OVER_CURRENT_MA, 100))
  Serial.println("INA226 : Not able to setAlert");

and:

void IRAM_ATTR handleInaAlert() { if (digitalRead(INA226_ALERT_PIN)) bIna226Alert = false; else bIna226Alert = true;

return; }

...

// Configure input of the INA226 Alert Pin (Open Collector/Drain) pinMode(INA226_ALERT_PIN, INPUT_PULLUP); attachInterrupt(digitalPinToInterrupt(INA226_ALERT_PIN), handleInaAlert, CHANGE); bIna226Alert = false;