SpenceKonde / megaTinyCore

Arduino core for the tinyAVR 0/1/2-series - Ones's digit 2,4,5,7 (pincount, 8,14,20,24), tens digit 0, 1, or 2 (featureset), preceded by flash in kb. Library maintainers: porting help available!
Other
551 stars 143 forks source link

TWI begin() does not fully implement swapped pins #223

Closed Taragorm closed 3 years ago

Taragorm commented 4 years ago

In twi.c#63

void TWI_MasterInit(uint32_t frequency)
{
    if(twi_mode != TWI_MODE_UNKNOWN) return;

    // Enable pullups just in case, should have external ones though
#ifdef NO_EXTERNAL_I2C_PULLUP
    pinMode(PIN_WIRE_SDA, INPUT_PULLUP);
    pinMode(PIN_WIRE_SCL, INPUT_PULLUP);
#endif

...

The pinMode() calls for pullups refer to the default pins, unless you mess with defining macros. This broke my code, as it assigned an ouput back to an input...

[IMO this shouldn't be done anyway, as the WPU is no where near enough, and this has the possibility of making it work unreliably, as opposed to not. That said, I assume it's for Arduino compatibility].

SpenceKonde commented 3 years ago

Good catch.

I'm with you 100% on "we shouldn't try to use the wimpy internal pullups asif they were suitable for use as I2C pullups, as, obviously, they're not. Really, I ought to include pads so the user can easily install pullups on the I2C pins. For Rev. C of my dev boards, I'm considering swapping out the UPDI and LED resistors for a single 1206 4-element resistor network (I love those things - fewer parts to place, cheap, and solder very reliably... say I swapped the autoreset resistor for a 4-element 4.7k one. Two in series gets you 10k for the R in the RC that generates the reset pulse, and the other two could be optionally connected to I2C lines with solder bridge jumpers (I don't know if you've seen the latest versions of my breakouts, but I've become very enamored of solder bridge jumpers to add little-used functionality for those who want it.... not only is there the classic reset enable one, there's a default-connected spot where you can reversibly disconnect every place where power can come into Vcc (regulator - for eliminating the quiescent current of a regulator not being used, UPDI 3-pin header, and serial header, plus there's one that connects the XDIR pin of USART0 to the CTS line, so you can use the FTDI header to conveniently connect an RS485 transciever... or if autoreset isn't enabled, there's... DIO I think I called it? that connects the DTR line to an I/O pin too - with that and CTXD bridged, you've got all you need to rig up some sort of flow control using that 6-pin header....

Anyway,, yeah, I probably should just gun that pinMode setting entirely, since those pullups are pretty much guaranteed not to work (at least not reliably, which is much worse than if it just didn't work . I try to avoid

SpenceKonde commented 3 years ago

To be clear, I didn't write that; it was like that when we ganked the libraries from the official core.