There is a bug where only mode == 0 can set the output mode. mode == 1 is trapped by the first if clause and never written. And a bitwise operations bug because config_status & 0xef can only set OUTS1:0 to 10 (digital PWM) if the register was already 10 or 11, not if it was 00 or 01.
Suggested improvements:
Added function documentation for consistency with rest of library
Bug fix: mode == 1 for analog output now works, removed nested if clause
Removed duplicate readOneByte() call, function now twice as fast
New feature: mode == 2 sets reduced range analog 10-90% (better for some ADCs)
Comments inside function to explain clearing/setting register bits
Shorthand assignment operator &=
Renamed function to setOutput, since "output" is one word
removed lowByte(), since by definition config_status is a single byte
Folded variable declaration into fewer lines
Bug fix: correct bitwise operations for set/clear bits in register
/*******************************************************
Method: setOutput
In: 0 for digital PWM
1 for analog (full range 0-100% of GND to VDD)
2 for analog (reduced range 10-90%)
Out: none
Description: sets output mode in CONF register.
*******************************************************/
void AMS_5600::setOutput(uint8_t mode)
{
int _conf_lo = _addr_conf+1; // lower byte address
uint8_t config_status = readOneByte(_conf_lo);
config_status &= 0b11001111; // bits 5:4 = 00, default
if (mode == 0) {
config_status |= 0b100000; // bits 5:4 = 10
} else if (mode == 2) {
config_status |= 0b010000; // bits 5:4 = 01
}
writeOneByte(_conf_lo, config_status);
}
There is a bug where only
mode == 0
can set the output mode.mode == 1
is trapped by the firstif
clause and never written. And a bitwise operations bug becauseconfig_status & 0xef
can only set OUTS1:0 to 10 (digital PWM) if the register was already 10 or 11, not if it was 00 or 01.Suggested improvements:
if
clausereadOneByte()
call, function now twice as fastlowByte()
, since by definition config_status is a single byte