I'm trying to do something pretty simple - pass a 2 byte int from my ATTiny85 to the Master using I2C. I've reviewed loads of examples on line and thought the code I had should be working. I can only get it to work if I uncomment the "int windcount = 12345;" line within the requestEvent(), even though windcount is declared at the top. As soon as I comment it out again I2C fails on the master.
Is anybody able to point out what I assume is a schoolboy error somewhere in my code?
Many thanks
Code for the Slave:
// Code for the ATtiny85
#define I2C_SLAVE_ADDRESS 0x4 // Address of the slave
#include <TinyWireS.h>
#include <avr/sleep.h>
#include <avr/interrupt.h>
const int rainPin = 3; // which pin is rain sensor connected to
const int windPin = 4; // which pin is wind sensor connected to
const int statusLED = 1; //which pin is status LED connected to
volatile int raincount = 0;
volatile int windcount = 0;
volatile int t=0;
byte lowWind;
byte highWind;
volatile String requestType = "";
volatile int response;
void setup()
{
TinyWireS.begin(I2C_SLAVE_ADDRESS); // join i2c network
TinyWireS.onReceive(receiveEvent);
TinyWireS.onRequest(requestEvent);
pinMode(rainPin, INPUT);
digitalWrite(rainPin, HIGH);
pinMode(windPin, INPUT);
digitalWrite(windPin, HIGH);
pinMode(statusLED, OUTPUT);
} // setup
void sleep() {
GIMSK |= _BV(PCIE); // Enable Pin Change Interrupts
PCMSK |= _BV(PCINT3); // Use PB3 as interrupt pin
PCMSK |= _BV(PCINT4); // Use PB4 as interrupt pin
ADCSRA &= ~_BV(ADEN); // ADC off
//WDTCR &= ~_BV(WDIE); // DISABLE WATCHDOG
set_sleep_mode(SLEEP_MODE_PWR_DOWN); // replaces above statement
sleep_enable(); // Sets the Sleep Enable bit in the MCUCR Register (SE BIT)
sei(); // Enable interrupts
sleep_cpu(); // sleep
cli(); // Disable interrupts
PCMSK &= ~_BV(PCINT3); // Turn off PB3 as interrupt pin
PCMSK &= ~_BV(PCINT4); // Turn off PB4 as interrupt pin
sleep_disable(); // Clear SE bit
ADCSRA |= _BV(ADEN); // ADC on
//WDTCR |= _BV(WDIE); // Watchdog on
sei(); // Enable interrupts
} // sleep
ISR(PCINT0_vect) {
// DO NOTHING HERE, CATCH EVERYTHING IN LOOP // Count rain gauge bucket tips as they occur
}
void loop()
{
if (digitalRead (rainPin) == LOW) {
raincount ++;
}
if (digitalRead (windPin) == HIGH) {
windcount ++;
}
// This needs to be here
TinyWireS_stop_check();
// Go back to sleep
sleep();
tws_delay(1);
}
// Gets called when the ATtiny receives an i2c request
void requestEvent()
{
//send raincount (max rain = 3" per minute)
TinyWireS.send(raincount);
//int windcount = 12345;
byte lowWind = (byte)(windcount & 0xff);
byte highWind = (byte)((windcount >> 8) & 0xff);
//t=0;
TinyWireS.send(lowWind);
TinyWireS.send(highWind);
//tws_delay(50);
}
// Gets called when the ATtiny receives an i2c write slave request
void receiveEvent(int howMany)
{
for( int i = 0; i < howMany; i++)
{
while(TinyWireS.available())
{
response = TinyWireS.receive();
if (response == 1) {
raincount = 0;
//requestType = "resetrain";
} else if (response == 2) {
windcount = 0;
//requestType = "wind";
} else {
// DO NOTHING
}
}
//tws_delay(100);
}
}
Code for the Master (extract):
void setup () {
//.......
Wire.begin();
while(!Serial) {} //Wait
int timeout=0;
while(!bme.begin()) {
Serial.println();
Serial.println("COULD NOT READ FROM I2C SENSORS, CHECK WIRING!!!!");
}
//.......
}
Needlerp:
As soon as I comment it out again I2C fails on the master.
Define what you mean by "fails" and post the code for the master too.
Please edit your post and put the code inside of code tags. Use the left most button </> in the forum editor to insert them.
Is anybody able to point out what I assume is a schoolboy error somewhere in my code?
May be because there is a smiley with glasses in your code?
Joke aside (Please use code tags) have you tried first copying your global volatile windcount variable into a local one ('with a different name) within a no interrupt section? Given you are messing around with the LSB and MSB of that int, it can change underneath you
Also why do you do TinyWireS.send(raincount); with an int ? And is the master waiting for 3 bytes?
If windPin is not held LOW, the global version of windcount will overflow in a few seconds in the loop() because you have activated the pull-up resistor on windPin.
In Master, when I uncomment the //int windcount = 12345; all is fine, but without it I can't event get as as far as bme.begin() - so I'm assuming I've done something dumb in my slave code.
For raincount I know that the values will be less than 255 so I haven't had a problem, whereas windcount could be up to 3000 - so I need to pass it properly (will go back and correct raincount once I get it working for correctness).
Not sure what you mean about copying global variable into a local one - can you give me a pointer?
Many thanks for the quick responses - total newbie here with good copy / paste skills from other code extracts but very little knowledge of what's actually going on (but a fast learner)!
I think you're on to something - I swapped the loop to catch the low rather than high (it's a reed switch wind speed sensor) and it works for a minute but then stops, but whilst it's working I do get readings so perhaps it wasn't the i2c causing the issue.
Is there anyway I can capture only a change (or falling edge) on the interrupt to avoid the overflow?
Needlerp:
In Master, when I uncomment the //int windcount = 12345; all is fine, but without it I can't event get as as far as bme.begin() - so I'm assuming I've done something dumb in my slave code.
The problem with only posting part of the master code is that, as a beginner, you can't judge what is important or not. In this case, you refer to bme.begin() but we have no idea what that is.
Better just to post all the code and let the folks here pick through it.
To identify changes in the reed switch, you have to save the old value in a variable, then compare the new and the old values. If these are different, Increment a counter.
How have you wired the reed switch? Between the Arduino pin and ground?
Yes, I wired the reed switch between PB4 and ground with an exernal 1k pullup resistor. The reed is normally closed.
I've put in a loop to catch changes as I think that was probably a secondary issue, but I'm still getting a script failure the minute I uncomment the line:
byte lowWind = (byte)(windcount & 0xff);
Yet this line is fine if I also in-comment the int windcount = 1234; line as well.
I think it's something to do with an error when trying to cast windcount as a byte, but due to my total ineptness I'm not sure how / why!
// Code for the ATtiny85
#define I2C_SLAVE_ADDRESS 0x4 // Address of the slave
#include <TinyWireS.h>
#include <avr/sleep.h>
#include <avr/interrupt.h>
const int rainPin = 3; // which pin is rain sensor connected to
const int windPin = 4; // which pin is wind sensor connected to
const int statusLED = 1; //which pin is status LED connected to
volatile int raincount = 0;
volatile int windcount = 0;
volatile bool thiswind;
volatile bool lastwind;
//volatile int t=0;
byte lowWind;
byte highWind;
volatile int response;
void setup()
{
TinyWireS.begin(I2C_SLAVE_ADDRESS); // join i2c network
TinyWireS.onReceive(receiveEvent); // not using this
TinyWireS.onRequest(requestEvent);
pinMode(rainPin, INPUT);
digitalWrite(rainPin, HIGH);
pinMode(windPin, INPUT);
lastwind = LOW;
digitalWrite(windPin, HIGH);
pinMode(statusLED, OUTPUT);
} // setup
void sleep() {
GIMSK |= _BV(PCIE); // Enable Pin Change Interrupts
PCMSK |= _BV(PCINT3); // Use PB3 as interrupt pin
PCMSK |= _BV(PCINT4); // Use PB4 as interrupt pin
ADCSRA &= ~_BV(ADEN); // ADC off
//WDTCR &= ~_BV(WDIE); // DISABLE WATCHDOG ***************PMN HERE*********************
set_sleep_mode(SLEEP_MODE_PWR_DOWN); // replaces above statement
sleep_enable(); // Sets the Sleep Enable bit in the MCUCR Register (SE BIT)
sei(); // Enable interrupts
sleep_cpu(); // sleep
cli(); // Disable interrupts
PCMSK &= ~_BV(PCINT3); // Turn off PB3 as interrupt pin
PCMSK &= ~_BV(PCINT4); // Turn off PB4 as interrupt pin
sleep_disable(); // Clear SE bit
ADCSRA |= _BV(ADEN); // ADC on
//WDTCR |= _BV(WDIE); // Watchdog on
sei(); // Enable interrupts
} // sleep
ISR(PCINT0_vect) {
// DO NOTHING HERE, CATCH EVERYTHING IN LOOP // Count rain gauge bucket tips as they occur
}
void loop()
{
// This needs to be here
TinyWireS_stop_check();
if (digitalRead (rainPin) == LOW) {
raincount ++;
}
thiswind = digitalRead(windPin);
if (thiswind != lastwind) { //check if value has changed since last time
if (thiswind == HIGH) { //value has changed - was it rising edge?
windcount ++;
}
lastwind = thiswind; //switch state ready for next time
}
//if (digitalRead (windPin) == HIGH) {
// windcount ++;
//}
// Go back to sleep
sleep();
tws_delay(1);
}
// Gets called when the ATtiny receives an i2c request
void requestEvent()
{
//send raincount (max rain = 3" per minute)
TinyWireS.send(raincount);
//int windcount = 0;
//byte lowWind = (byte)(windcount & 0xff);
//byte highWind = (byte)((windcount >> 8) & 0xff);
//t=0;
//TinyWireS.send(lowWind);
//TinyWireS.send(highWind);
TinyWireS.send(windcount);
TinyWireS.send(windcount);
//tws_delay(50);
}
// Gets called when the ATtiny receives an i2c write slave request
void receiveEvent(int howMany)
{
for( int i = 0; i < howMany; i++)
{
while(TinyWireS.available())
{
response = TinyWireS.receive();
if (response == 1) {
raincount = 0;
} else if (response == 2) {
windcount = 0;
} else {
// DO NOTHING
}
}
//tws_delay(100);
}
}
[code]
Sorry, it's an ATTiny85 so I can't debug it directly. I'm using I2C to get the data as per the script. When it works fine my master script reads raincount and windcount and sends them to my serial debugger. As soon as I change the lines I refer to above I never get past the following in my Master:
I'm also reading a BME sensor through I2C, so bme referred to below is called via BME280I2C bme; before startup .
Wire.begin();
while(!Serial) {} //Wait
int timeout=0;
while(!bme.begin()) {
Serial.println();
Serial.println("COULD NOT READ FROM I2C SENSORS, CHECK WIRING!!!!");
Serial.println();
break; // Until reset is worked out, just break and carry on...
delay(500);
timeout++;
if (timeout == 10) {
Serial.println("Sending reset command to Attiny85");
digitalWrite(D3, LOW); // Send Reset command to ATtiny85
delay(500);
digitalWrite(D3, HIGH);
}
}
Not sure how they're linked, but could it be something about pullup resistors on the Attiny85? I've just read somewhere about having 4.7kOhm pullups which I don't have?
Assuming it is still true that forcing the global variable windcount to the value 12345 makes everything appear to work, then you should focus on that for the moment.
You could try the following:
Change the function requestEvent() to the following:
void requestEvent()
{
//send raincount (max rain = 3" per minute)
TinyWireS.send(raincount); // !! raincount is an int. send() requires a byte.
//int windcount = 12345; // global var . check also if uncommenting this makes it work OK.
//t=0;
TinyWireS.send( lowByte( windcount ) ); // ensure that the master is expecting
// this order low byte first, then high byte
TinyWireS.send( highByte( windcount ) );
//tws_delay(50);
}
You could also remove the call to sleep() in the loop. I believe that is not relevant here, since you have moved away from the 'interrupt' solution to the 'polling' solution, at least according to the relics you've left in the code.
If it is battery powered, and battery life time is a problem, you could move back to the interrupt solution once you have something working.
OK thanks for your help and support. I suspect that due to my lack of experience there will be a number of different factors all contributing to the problem, and it's difficult for me to work out which.
I did realise last night that forcing the global variable windocunt to 12345 does make it work, but only for a couple of minutes and then it crashes - I hadn't been leaving it long enough to check that it was truly stable. Interestingly, before I started adding in the second sensor, I don't recall having any issues with the raincounter, but suspect I was still having issues.
I do want it to run on battery eventually so will need the interrupts / sleep, but need to walk before I can run I think!
I think I'll go back to basics, try an really simple counter script on the I2C to check whether the slave to master connection is robust running purely with the I2C pullups on by BME breakout board, and then go back to gradually building in complexity back into the slave script.
Thanks to your pointers, I managed to get this sorted. I'm not sure whether it was one or all of the below but between them I've now had I2C running stably for several days:
Using TinyWireS, only send one byte of data back to the master per request. I've split the data into three chunks and request 1 byte three times on the master, then on the slave iterate through them.
Make sure that the interrupts catch a change in position (from high to low or vice versa) to avoid overflows.
I also changed the clock speed on my attiny85 to 8mhz.
