Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slave onRequest send more than one byte #12

Open
RuslanasMobOn opened this issue Jun 8, 2018 · 11 comments
Open

Slave onRequest send more than one byte #12

RuslanasMobOn opened this issue Jun 8, 2018 · 11 comments

Comments

@RuslanasMobOn
Copy link

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
Copy link

tatobari commented Jun 8, 2018

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
Copy link

RyanLoringCooper commented Jun 8, 2018

@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
Copy link

tatobari commented Jun 8, 2018

@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
Copy link

RyanLoringCooper commented Jun 8, 2018

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
Copy link

tatobari commented Jun 8, 2018

@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
Copy link

RyanLoringCooper commented Jun 8, 2018

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
Copy link

tatobari commented Jun 8, 2018

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

@RyanLoringCooper
Copy link

@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
Copy link
Owner

@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
Copy link

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

@ChrisHul
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants