Tympan / Tympan_Library

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

Wrong pin on micro specified for AIC reset #40

Closed eyuan-creare closed 2 years ago

eyuan-creare commented 2 years ago

@chipaudette I believe the wrong Tympan pin is assigned to reset the AIC , not that it matters. When the AIC boots up it will reset to default anyway.

Here is the Rev-D board showing pin A8 is connected to nReset

Teensy 3.6 diagram shows A8 is pin#22 image

Reset pin defined as pin#21 on Teensy 3.6 https://github.com/Tympan/Tympan_Library/blob/c44b8c3d6fce48084498e54583e6e4b5d78af186/src/control_aic3206.h#L45

image

Digital write called to reset the AIC image

Looks like this was never got updated from the Teensy 3.2
image

biomurph commented 2 years ago

@eyuan-creare @chipaudette Eric you are looking at a reference to the ball array position. The pin number is A8 as you say, but that is only reference to the pin number of the BGA Note in the schematic that I have listed the Teensy pin numbers in BLUE alongside the BGA pin numbers.

Screen Shot 2021-07-05 at 12 20 56 PM
eyuan-creare commented 2 years ago

@biomurph Thanks for taking a look and straightening me out. That makes sense. So is it okay that nReset is assigned to Pin21?

define AIC3206_DEFAULT_RESET_PIN 21

biomurph commented 2 years ago

The nRESET pin is connected to Teensy pin 35 on Rev D

chipaudette commented 2 years ago

In practice, the nReset pin actually gets chosen and set by the creation of the Tympan class (see Tympan.h). I believe that it overwrites resetPinAIC away from the default set by this #define and to whatever pin is specified in Tympan.h for whichever rev you are using. So, in effect, this #define ends up getting ignored.

I created Tympan.h as my way of handling the diff Tympan revs. Since I did this way back for the release of RevD, I bet that #define value is what was correct for the RevA/RevC design. It's like you found a fossil!

I bet there are many more confusing fossils in there like this one. Unfortunately.

Feel free to add comments to the code to make it less confusing, if you think it'll help.

chipaudette commented 2 years ago

With your work on the AIC3212, we have to decide whether to fold it into Tympan.h as another variant of Tympan, or whether we create a whole new class.

Given that Tympan.h extends AIC3206, it seems like switching to 3212 breaks that underlying foundational assumption. We could abstract the interaction with the AIC to allow either AIC to be handled by the Tympan class, but that only makes sense if the two AICs offer similar functionality. I'm not sure that they do.

I think that creating a new AIC3212 class (modeled on the 3206 class...or not...whatever) is probably the right path.