lucullusTheOnly / TinyWire

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

Slave `onRequest` send more than one byte #12

Open RuslanasMobOn opened 6 years ago

RuslanasMobOn commented 6 years ago

Hi, is it possible to send more than one byte of data per one request? My current code is:

void requestEvent()
{
    int8_t response = 123;
    TinyWire.send(response);
}

void setup() {
  TinyWire.begin(I2C_SLAVE_ADDRESS);
  TinyWire.onRequest(requestEvent);
}

void loop() {

}
tatobari commented 6 years ago

I can't believe I know this one. The onRequest callback is run among a function that takes care of handling the I2C end of response protocol,

You would do the following, adding as many TinyWire.send(foo) as you need. At least, that worked for me. @lucullusTheOnly might have corrections to make.

void onRequestHandler()
{
    int8_t responseByte1 = 123;
    int8_t responseByte2 = 112;
    TinyWire.send(responseByte1);
    TinyWire.send(responseByte2);
}

void setup() {
  TinyWire.begin(I2C_SLAVE_ADDRESS);
  TinyWire.onRequest(onRequestHandler);
}

void loop() {

}
RyanLoringCooper commented 6 years ago

@tatobari I tried the exact same thing a while back, and unfortunately it is unreliable. In my experience it worked on the first 2 bytes, but not the third, and then on the fourth, but not the fifth or sixth. If somebody really wants I can link to the code that I was using to get this to happen, but it is hard to test as it was communicating with a flow sensor and sending the information it got over i2c to another device.

tatobari commented 6 years ago

@RyanLoringCooper I think sharing the code might be a good idea. The onRequest callback runs during an interruption. It must be kept realtively small and you must make sure you already have the information saved in the controller at the moment the callback is run. Otherwise you might end up in an unpredictable behaviour caused by other interruptions running while you're handling another interruption.

In other words, the callback must be as simple and small as possible.

RyanLoringCooper commented 6 years ago

Hopefully it helps. I am willing to answer questions.

    // SCL on physical pin 9 (PA4)
    // SDA on physical pin 7 (PA6)
    // sensor on physical pin 3 (PB1)
    #include <avr/interrupt.h>
    #include <avr/io.h>
    #include <avr/pgmspace.h>
    #include "TinyWire.h"

    // this is for physical pin 3, PB1, pin 8 if using pinMode(2) or other convience functions
    #define SENSOR_PIN PB1
    #define mAddr 0x0A
    #define STRING_LENGTH 5

    volatile int blipsSeen = 0;
    long lastTime = 0;
    float flow = 0;
    volatile char flowString[STRING_LENGTH];

    ISR(INT0_vect) {
        blipsSeen++;
    }

    void i2cISR() {
        TinyWire.send(flowString[0]);
        TinyWire.send(flowString[1]);
        TinyWire.send(flowString[2]);
        TinyWire.send(flowString[3]);
        TinyWire.send(flowString[4]);
        TinyWire.send(flowString[5]);
    }

    void setupFlowSensor() {
        blipsSeen = 0;
        DDRB    &=  ~(1<<SENSOR_PIN);       // enables SENSOR_PIN to be an input
    //    PUEB  |=  1<<SENSOR_PIN;          // enables a 10kOhm pullup resistor on SENSOR_PIN
        pinMode(8, INPUT_PULLUP);
        PCMSK1  |=  1<<SENSOR_PIN;          // enables SENSOR_PIN to be a pin listening for interrupts
        GIMSK   |=  1<<INT0;                // enables INT0 interrupt
        MCUCR   |=  1<<ISC01 | 1<<ISC00;    // enables only rising edge to trigger INT0 
        SREG    |=  1<<7;  // enables interrupts
    }

    void estimateFlow() {
        // if 1000ms have passed or the millis counter rolled over
        if(millis()-lastTime > 1000 || millis() < lastTime) {
            // rightshift to divide by two because isr is triggered on a change, but we want rising
            int blips = blipsSeen;
            flow = blips/4.8;
            dtostrf(flow, 4, 2, flowString);
            //sprintf(flowString, "%d", blips);
            blipsSeen = 0;
            lastTime = millis();
        }
    }

    void setup() {
        setupFlowSensor();
        TinyWire.begin(mAddr);
        TinyWire.onRequest(i2cISR);
    }

    void loop() {
        estimateFlow();
    }
tatobari commented 6 years ago

@RyanLoringCooper Would you mind enclosing your code with the following syntax?

` ` `c++
// Code here
` ` `

(I've added spaces between each of the three french accents to prevent the parser from converting them into a block code.)

RyanLoringCooper commented 6 years ago

I guess it is worth mentioning that to solve the problem that I mentioned I turned i2cISR() into

// This should be called STRING_LENGTH number of times in quick succession.
void i2cISR() {
    TinyWire.send(flowString[i2cIndex++]);
    if(i2cIndex == STRING_LENGTH) 
        i2cIndex = 0;
}

The ATTiny84 that this code was running on was communicating with a Raspberry Pi using the i2c_smbus commands.

And thanks for telling me about the c++ syntax highlighting.

tatobari commented 6 years ago

@lucullusTheOnly will probably be able to help you with that. Your approach works for me but using only 3 bytes.

RyanLoringCooper commented 6 years ago

@tatobari so you are seeing the issue I mentioned, or you only tested sending 3 bytes? Which approach are you referring to?

There is a chance that my problem was caused by static inline __s32 i2c_smbus_read_block_data(int file, __u8 command, __u8 *values) which is what the Pi was using when I had the 6 send statements in i2cISR()

lucullusTheOnly commented 6 years ago

@RuslanasMobOn : tatobari's approach should be the correct way of sending more than one byte for a request. The send() function just fills the output buffer, which is then send, until it is empty or the master sends a NACK.

@RyanLoringCooper : I don't really know, what causes your issue and I couldn't find a problem, when I reviewed the corresponding part of the code just now. Maybe this is some weird timing issue with the interrupt for the flow sensor. Can you test your code with the flow sensors interrupt disabled? If the multiple send()s don't make a problem then, it would confirm this suspicion.

EDIT: @RyanLoringCooper : Since you posted during me writing this comment: Can you also check the same code with a normal Arduino?

RyanLoringCooper commented 6 years ago

@lucullusTheOnly I'll give it a try when I can. I'm not totally convinced that my problem was caused by your library.

ChrisHul commented 2 years ago

I've spent some time looking at this problem because I was unable to make ANY TinyWire work. The main issue is that I2C specification does not allow SCL stretching between the data byte and ACK. This means the ATTiny needs to act very quickly to setup the ACK reading after the data byte has been shifted. Even a minor change in the software may have an impact on performance. I also discovered that the clock was released concurrently with the USI counter loading which in my view is compromising. I adapted a master bitbang software to accept SCL stretching at any time which allows sending streams of data in both directions. It still has a failing ratio of 0.1 % in master to slave transfer, but it should be ok with an extra layer of CRC and handshaking on top. I'm unable to explain why these errors occur. For anyone interested in this setup: https://github.com/ChrisHul/TinyWireSetup