Tympan / Tympan_Library

Arduino/Teensy Library for Tympan Open Source Hearing Aid
MIT License
124 stars 33 forks source link

Wrong register values for Mic Bias #41

Open eyuan-creare opened 3 years ago

eyuan-creare commented 3 years ago

Value for 1.7V mic bias is not offset to the correct bits

Here is the register for Mic Bias (AIC3206), showing bias voltage as bits D5-D4. For example, to set Mic Bias to 1.7V, we need TYMPAN_MIC_BIAS_POWER_ON | 0b0001 0000

image

@chipaudette perhaps you can double-check this, then I will branch off to make these corrections. Does this mesh with the behavior you've seen?

eyuan-creare commented 3 years ago

I don't think it is worth changing for the AIC3212, but I would typically use enums here. This allows coders to quickly look up the enum from the function's input argument, and ensures that only the correct range of values are used. With #defines, it is difficult to search for which define to use, unless you go off an example.

//names to use with setMicBias() to set the amount of bias voltage to use
#define TYMPAN_MIC_BIAS_OFF             0
#define TYMPAN_MIC_BIAS_1_25            1
#define TYMPAN_MIC_BIAS_1_7             2
#define TYMPAN_MIC_BIAS_2_5             3
#define TYMPAN_MIC_BIAS_VSUPPLY         4
#define TYMPAN_DEFAULT_MIC_BIAS TYMPAN_MIC_BIAS_2_5

to

typedef enum {
    off, 
    Mic_Bias_1_25V, 
    Mic_Bias_1_70V,
   ...
} Mic_Bias_Values_t;

then in the function...

bool AudioControlAIC3212::setMicBias(Mic_Bias_Values_t biasSetting) {
  switch (biasSetting){
        case Mic_Bias_1_25V:
           aic_writeAddress(TYMPAN_MIC_BIAS_REG, TYMPAN_MIC_BIAS_POWER_ON | TYMPAN_MIC_BIAS_OUTPUT_VOLTAGE_1_25); // power up mic bias
          break;
    }
}




To take it a step further, I would encode the register value for mic bias voltage like this:

typedef enum {
    uint8_t Mic_Bias_1_25V = 0x00, 
    uint8_t Mic_Bias_1_70V = (0x01<<4) ,
    ...
} Mic_Bias_Voltage_t;

then in the function:

bool AudioControlAIC3212::setMicBias(Mic_Bias_Voltage_t biasSetting) {
    uint8_t regValue = 0;

    // Read in what's there to preserve the other bits 
   regValue  =  aic_ReadAddress(TYMPAN_MIC_BIAS_REG);

   // Mask out the desired bits to change
   regValue &= (~MIC_BIAS_VOLTAGE_BIT_MASK); 

    // Write the new register value
    aic_writeAddress(TYMPAN_MIC_BIAS_REG, regValue | biasSetting);
}

I think this is much cleaner.

eyuan-creare commented 3 years ago

In regards to the AIC3212, it uses different mic bias voltage. Is that an issue with any of our mics?

image

chipaudette commented 3 years ago

We've only ever used the full bias voltage (2.2v?), so these other voltage values have never ended up being an issue. It's good to get them correct, but it's not yet been revealed as a problem die to always using the full voltage.

I agree that an enum send better. Someone else did the initial code where #define was used instead of enum. Enum seems better.

Chip