lucullusTheOnly / TinyWire

Composite Master and Slave I2C library for Atmels ATTiny microcontrollers
107 stars 26 forks source link

Does this work with ATtiny84? #7

Closed markboydcode closed 6 years ago

markboydcode commented 6 years ago

When I attempt using this library in the code below I get copious error like the following. Does this library work with the tiny84? I don't see checks for the 84 in the twi.h file. (I am using the Arduino IDE and have the ATtiny84 selected as the processor.) Commenting out the TinyWire lines does compile and upload to the processor just fine.

/Users/boydmr/Documents/Arduino/libraries/TinyWire/twi.cpp: In function 'void SET_USI_TO_SEND_ACK()':
/Users/boydmr/Documents/Arduino/libraries/TinyWire/twi.cpp:107:3: error: 'DDR_USI' was not declared in this scope
   DDR_USI |= ( 1 << PORT_USI_SDA );
   ^
/Users/boydmr/Documents/Arduino/libraries/TinyWire/twi.cpp:107:21: error: 'PORT_USI_SDA' was not declared in this scope
   DDR_USI |= ( 1 << PORT_USI_SDA );
                     ^

My test code is the following:

#include <TinyWire.h>
int led1 = PA1;
int led2 = PA2;
int btn = PA0;
int last = -1; 

void setup() {
  TinyWire.begin(); // join i2c bus - no address for master
  pinMode(led1, OUTPUT);
  pinMode(led2, OUTPUT);
  pinMode(btn, INPUT_PULLUP);
}

void loop() {
  int current = digitalRead(btn);
  if (last != current) { // only communicate changes
      last = current;
      TinyWire.beginTransmission(0); // transmit to device #0 which should be broadcast
      TinyWire.send(current);        // sends one byte
      TinyWire.endTransmission();    // stop transmitting

      if (current == 1) {
        digitalWrite(led1, HIGH); // led1 follows btn while led2 is !(btn)
        digitalWrite(led2,LOW);
      }
      else if (current == 0) {
        digitalWrite(led1,LOW);
        digitalWrite(led2, HIGH);
      }
  } 
  delay(5);
}
lucullusTheOnly commented 6 years ago

The ATTiny84 is currently not supported out of the box. But since the 84 also has a USI hardware, it should be easily enabled by adding the corresponding defines for the 84 in twi.h in the section "device dependent definitions". Currently the full set of definitions is only available for ATTiny25/45/85, so you can take this as orientation.

I would be glad to get a pull request, if you do that. I simply hadn't enough time for searching in other datasheets for the defines :)

markboydcode commented 6 years ago

I anticipated as much. I'll give it a try and verify working order before issuing the pull. Thanks.

markboydcode commented 6 years ago

It looks like this will be non-trivial due to the 84 having two ports A & B and SCL & SDA are each on a different port. So lines like this in code:

DDR_USI &= ~(( 1 << PORT_USI_SCL ) | ( 1 << PORT_USI_SDA )); // set SDA & SCL as input

won't work since there are separate registers being used. I'll see how I can extend your code to handle this and you can take a look in the pull req. But give me a while. I can only put in a little time here and there.

lucullusTheOnly commented 6 years ago

I looked up the data sheet of the ATTiny84 and I found, that SDA and SCL ARE on the same port. SCL is on PA4 and SDA is on PA6. Maybe you have mistaken the CLKI (Clock In) for SCL.

With them being on the same port, it should be easy to set the right definitions. Maybe I have time tomorrow, to look into this. But you have to test it, since I don't have an 84 here.

markboydcode commented 6 years ago

yes. I noticed that last week when attempting to go through the code. But I thought I had set the values correctly and wasn't able to get it working. So I'll pull your changes and give it a try when you are ready.

lucullusTheOnly commented 6 years ago

I created a new branch named "issue7". I added the definitions for ATTiny24/44/84 as shown in the datasheet and also changed/added some definitions for ATTiny25/45/85 and in "twi.cpp" to make it easier to adapt to other chips. It compiles fine for ATTiny84.

Try it out and if it works, I will merge the branch with master.

markboydcode commented 6 years ago

Cool. Will try it out tonight and let you know. Thanks.

markboydcode commented 6 years ago

I can't seem to make it work. I'm turning on an LED if endTransmission is non zero as in your master sender example and it is going on. So I may have something wrong with my circuit. Are we using internal pullups for SDA and SCL or must I add external pullups?

lucullusTheOnly commented 6 years ago

Yes, you should use external pullups, despite also using internal pullups. Referring to TinyWireM it isn't as forgiving as the Wire library. Can you check, which error code is returned? For example you can use a function that shows the error number with blinks:

void show_error(byte error_code){
    for(byte i=0;i<error_code;i++){
        digitalWrite(led_pin,HIGH);
        delay(500);
        digitalWrite(led_pin,LOW);
        delay(500);
    }
}
markboydcode commented 6 years ago

Added that method and the code was 3. Looked through the code base and I think I found that it was: USI_TWI_BAD_MEM_READ, "// Generated Start Condition not detected on bus". Then added external pullups and it still gave the same issue. would it help if I provided a schematic and the full code for both master and slave?

lucullusTheOnly commented 6 years ago

Yes, please share your code and wiring. Are you using a minimal code with only the TinyWire library? Make sure, that the pins are not used by other code and that your slave does not fiddle unwanted with the lines. Try a simple master (ATTiny84) and slave example. Also see what happens, if you comment out SIGNAL_VERIFY in twi.h, in case that this error is falsely triggered. (You can find the error generating line in twi.cpp in the function USI_TWI_Master_Start() )

markboydcode commented 6 years ago

It works. While writing down my circuit I found a wire out of place pulling one of the signals to ground. Once removed everything works fine. I did try again without the external pull-ups on SDA and SCL and the code 3 error returned. So the external pull-ups are definitely necessary. I also validated that the I2C broadcast address of 0 is received by all slaves while individual slaves respond appropriately to their assigned addresses.

Thanks for writing this and adding the values for the 84.

lucullusTheOnly commented 6 years ago

No problem. I will merge the corresponding branch with master.

I am currently working on another problem with multimaster. There may be a common reason for both scenarios, which explains, that external pullups are required. Maybe there is a way to make it work using only the internal pullups.

Since your problem is solved, I will close this issue.