Atemga328P As I2C Slave Sends Squares

I have an atmega 328p connected to a Micro as slave over I2C, the 328 is meant to measure two LDRs and send the results, however all I get are squares, the ascii code looks like 254 or something. Before the current code I had blink working, as well as 'hello ' which you can see commented out still in the code, these all worked fine, what am I doing wrong? No matter how many bits I request I just get back the requested number of squares.

Code:
Master

// Wire Master Reader
// by Nicholas Zambetti <http://www.zambetti.com>

// Demonstrates use of the Wire library
// Reads data from an I2C/TWI slave device
// Refer to the "Wire Slave Sender" example for use with this

// Created 29 March 2006

// This example code is in the public domain.


#include <Wire.h>

void setup() {
  Wire.begin();        // join i2c bus (address optional for master)
  Serial.begin(9600);  // start serial for output
}

void loop() {
  Wire.requestFrom(0x08, 2);    // request 2 bytes from slave device 0x08

  while (Wire.available()) { // slave may send less than requested
    int c = Wire.read(); // receive a byte as character
    Serial.println(c);         // print the character
  }

  delay(2500);
}

Slave

// Wire Slave Sender
// by Nicholas Zambetti <http://www.zambetti.com>

// Demonstrates use of the Wire library
// Sends data as an I2C/TWI slave device
// Refer to the "Wire Master Reader" example for use with this

// Created 29 March 2006

// This example code is in the public domain.


#include <Wire.h>

const byte SLAVE_ADDRESS = 0x08;
const int ldrPin1 = 17;
//const int ldrPin2 = 16;

void setup() {
  
  Wire.begin(SLAVE_ADDRESS);                // join i2c bus with address SLAVE_ADDRESS
  Wire.onRequest(requestEvent); // register event
  pinMode(ldrPin1, INPUT);
  //pinMode(ldrPin2, INPUT);
}

void loop() {
  delay(100);
}

// function that executes whenever data is requested by master
// this function is registered as an event, see setup()
void requestEvent() {
  int ldrStatus1 = analogRead(ldrPin1);
  //int ldrStatus2 = analogRead(ldrPin2);
  //Wire.write("hello "); // respond with message of 6 bytes
  Wire.write(ldrStatus1);
  //Wire.write(ldrStatus2);
}

Thanks for any help you can throw my way!

The slave is sending an int (2 bytes on an 8bit platform such as the Micro). You are printing each individual byte of that int. You must assemble the two bytes back into an int then print it.

You need to explicitly tell Wire.write how many bytes to send, otherwise it will send a single byte, with the exception that a string (char array) will send until it encounters the null terminator.

void requestEvent() {
  int ldrStatus1 = analogRead(ldrPin1);
  Wire.write((byte*)&ldrStatus1, sizeof(ldrStatus1));
}

requestEvent is an interrupt handler - DO NOT use Serial.print, it can lock up the code if the Serial transmit buffer is full.
Only use a single Wire.write in requestEvent - if you need to send multiple values store them in a struct or array and send all at once.

Ah ok, I have some code to chop the data into two pieces, however now all I get is '-1' repeated.
Code:

Slave

#include <Wire.h>

const byte SLAVE_ADDRESS = 0x08;

void setup() {
  
  Wire.begin(SLAVE_ADDRESS);                // join i2c bus with address SLAVE_ADDRESS
  Wire.onRequest(requestEvent); // register event
}

void loop() {
  delay(100);
}

void requestEvent()
{
int16_t bigNum = 1234;
byte myArray[2];
 
myArray[0] = (bigNum >> 8) & 0xFF;
myArray[1] = bigNum & 0xFF;
Wire.write(myArray, 2);
}

Master

#include <Wire.h>


void setup() {
  Wire.begin();        
  Serial.begin(9600);  
}

void loop() {
  delay(2500);
  
  int16_t bigNum;
  byte a, b;
  Wire.requestFrom(0x08, 2);    
 
   a = Wire.read(); 
   b = Wire.read();
   bigNum = a;
   bigNum = bigNum << 8 | b;
    
    
  //}
  Serial.println(bigNum);         
  Serial.println("\n");
}

Do you think that everyone knows the C++ Operator Precedence - cppreference.com ?

Could you test if you read something ?

void loop() 
{
  delay(2500);
  
  int16_t bigNum;
  int n = Wire.requestFrom(0x08, 2);    
  if (n==2)
  {
    byte a = (byte) Wire.read(); 
    byte b = (byte) Wire.read();
    bigNum = (int16_t) word( a, b);
    Serial.println(bigNum);         
    Serial.println("\n");
  }
  else
  {
    Serial.println("Error");
  }
}

Thanks for the help, the code above when run does not trigger the "Error", so that part is ok. It still prints -1s, interestingly the /n gives 2 line feeds not just one as I thought it would, see below:

18:33:16.213 -> -1
18:33:16.213 -> 
18:33:16.213 -> 

As to the question of not explaining C++ operators my apologies, I always assume other users here know way more than me, so I didn't think I needed to. The link is quite informative and will be a handy guide going forward, thank-you.

OK, things are getting very strange, I no longer think this issue has anything to do with the code, originally this IC was going to measure light via 2 LDRs, I still had them wired up throughout the above comments, while waiting I removed one to play with a different project, well the values in the serial monitor went from solid -1 to all kinds of random numbers. See below:

20:05:04.524 -> -1
20:05:04.524 -> 
20:05:04.524 -> 
20:05:07.044 -> -8961
20:05:07.044 -> 
20:05:07.044 -> 
20:05:09.525 -> -8449
20:05:09.525 -> 
20:05:09.525 -> 
20:05:12.045 -> 31487
20:05:12.045 -> 
20:05:12.045 -> 
20:05:14.525 -> 23295
20:05:14.525 -> 
20:05:14.525 -> 
20:05:17.045 -> 26623
20:05:17.045 -> 
20:05:17.045 -> 
20:05:19.565 -> -28673

This should not be happening if the code mentioned above had been loaded, it looks like the pins are still set as inputs, whereas the code has the value hard coded in it.

So some background, I've had nothing but issues getting this 328p to cooperate, bootloading was a nightmare. I followed instructions exactly for using my Uno as an ISP and triple checked wiring, even had local electronics group look at photos to confirm but I still got the dreaded "avrdude: Yikes! Invalid device signature" message, I tried numerous combinations of settings and doing the instructions in different order...
One of the recommendations from the local guys was to replace the breadboard and wiring, incidentally while doing this I also moved the crystal nearer the IC, suddenly it bootloaded successfully!! I thought I was in the clear! But now it seems it is back, I can load all the sketches I want successfully but the 328 behaves the same. I cannot bootload it either, when I try to do so the Uno disappears off the serial connection and needs to be unplugged and plugged back in to respond, then for some combinations I get the error below:

avrdude: stk500_recv(): programmer is not responding
avrdude: stk500_getsync() attempt 1 of 10: not in sync: resp=0x03

Which counts up to attempt 10 and fails. This is so weird, I really don't know what the issue is, I know the connections are fine as I am using all brand new wire and breadboard.

Also unrelated to all of this I realize but it sounds like a thunder and lightning storm is brewing outside! I have awoken something with this IC it seems.

So that was a long response, any questions? Are there more details I need to add? And most importantly, what am I doing wrong???

After doing this in the master:

Print the hex values of a and b :
Serial.println( a, HEX) ;
Serial.println( b, HEX) ;
To see if that shows where it goes wrong.

Edit
Post crossed with the OP.

The Serial.println() prints a CarriageReturn + LineFeed.
Your Serial.println("\n") prints the text, which is a LineFeed, followed by CarriageReturn + LineFeed.

There can be maybe hundred problems with a bare ATmega chip and the I2C bus. Suppose you did 90 of those correctly, then you still have 10 problems.

Jumper wires can be broken and breadboards have bad contacts, especially if they are new and not used yet. The most common problem is missing decoupling capacitors. Another problem is that the GND side of the 22pF capacitors should be very close to the ATmega chip. Or a very cheap crystal, or not connecting all the GND and (A)VCC pins, or connecting AREF to VCC, and so on. That is just the ATmega chip. The I2C bus has its own problems, such as missing pullup resistors, using flat ribbon cable, missing GND connection.

Can you show a photo ?

There are no decoupling capacitors. Put a few 100nF capacitors near the ATmega chip at GND and VCC.

Just to verify, do you have the capacitor between RESET and GND on the UNO being used as the ISP programmer?

There is a 10uF cap across ground and RESET yes. I've added a 100nF cap across both physical pins 7/8 and 20/22, no change. It is very odd that I can upload code fine, the code as written doesn't initialize any pins, merely playing with variables internally yet the data sent is coming from a pin? If I already have a bootloader loaded, loading another merely replaces the old correct?

Can you post a picture of the wiring used while running the I2C tests?
You do have a ground wire connected as well as the SCL and SCA lines?
Do you have pullup resistors connected to the SCL and SCA lines?

Well, suddenly things are working fine! I disconnected everything on the Uno and reuploaded the ISP sketch, then reconnected all the wires and after two fails the slave sketch above loaded. I was skeptical as it has said everything is fine before, I moved the 328 back to the Micro breadboard and the serial monitor has the most wonderful repeating 1234, 1234, 1234, no -1, no random numbers, but 1234!!! I wonder if the additional caps helped and reloading the ISP sketch cleared some kind of reset I may have inadvertently done. Now off to get the LDRs to pass their data to the Micro, should be easy right?! :wink:

@GaGa111 I have adjusted our Master and Slave skethes as follows; execute them and report the results.

// Wire Master Reader Codes

#include <Wire.h>
byte arrayC[2];
int i = 0;

void setup() 
{
  Wire.begin();        // join i2c bus (address optional for master)
  Serial.begin(9600);  // start serial for output
}

void loop() 
{
  Wire.requestFrom(0x08, 2);    // request 2 bytes from slave device 0x08

  while (Wire.available()) 
  { // slave may send less than requested; 
      arrayC[i]  = Wire.read(); // receive a byte as character
      Serial.println(arrayC[i], HEX);    //as you are receivivg hex numbers and not charcaters
      i++;
  }
  int x = arrayC[0]<<8|arrayC[1];  //forming 16-bit
  Serial.println(x, HEX);    //showing the value of ldeStatus1 sent by Slave
  i = 0;

  delay(2500);
}

Slave Codes

#include <Wire.h>

const byte SLAVE_ADDRESS = 0x08;
const int ldrPin1 = 17;

void setup() 
{
  
  Wire.begin(SLAVE_ADDRESS);                // join i2c bus with address SLAVE_ADDRESS
  Wire.onRequest(requestEvent); // register event
}

void loop() 
{
  delay(100);
}

void requestEvent() 
{
  int ldrStatus1 = analogRead(ldrPin1);
  Wire.write(highByte(ldrStatus1));   //I2C is byte oriented bus; it delas 8-bit block at a time
  Wire.write(lowByte(ldrStatus1));    //lower byte is sent next
}

I do not have anything set up to test this:

Master code:

#include <Wire.h>

const byte SLAVE_ADDRESS = 0x08;

void setup()
{
  Wire.begin();        // join i2c bus (address optional for master)
  Serial.begin(9600);  // start serial for output
}

void loop()
{
  int ldrStatus[2];
  if (Wire.requestFrom(0x08, sizeof(ldrStatus)) == 0)
  { 
    //return value of 0 indicates an error
    Serial.print(F("Error"));
  }
  else
  { 
    //Wire.requestFrom will ALWAYS return the number of bytes requested if no error occurs
    //  regardless of the number of bytes actually sent by the slave device
    byte* p = (byte*) &ldrStatus;
    for (size_t i = 0; i < sizeof(ldrStatus); i++)
    {
      *p++ = Wire.read();
    }
    Serial.print(F("ldrStatus1 = "));
    Serial.println(ldrStatus[0]);
    Serial.print(F("ldrStatus2 = "));
    Serial.println(ldrStatus[1]);
  }

  delay(2500);
}

Slave code:

#include <Wire.h>

const byte SLAVE_ADDRESS = 0x08;
const byte ldrPin1 = 17;
const byte ldrPin2 = 16;

void setup()
{
  Wire.begin(SLAVE_ADDRESS);                // join i2c bus with address SLAVE_ADDRESS
  Wire.onRequest(requestEvent); // register event
}

void loop()
{
  delay(100);
}

void requestEvent()
{
  int ldrStatus[2];
  ldrStatus[0] = analogRead(ldrPin1);
  ldrStatus[1] = analogRead(ldrPin2);
  Wire.write((byte*)&ldrStatus, sizeof(ldrStatus));
}
1 Like

Suggestion to simplify your circuit: you don't need the crystal and it's caps. The atmega will run fine at 8MHz on its internal oscillator. I recommend using the MiniCore Arduino core.

david_2018, your code works perfectly! PaulRB, I'll try your idea, I do not need the accuracy that the 16Mhz crystal brings and this would be one less point of failure in the future, by failure I'm implying my wiring ;). Another discovery is that having the serial window open causes issues, something to do with the IDE tricking avrdude into thinking there is another device connected, so going forward I always make sure it is closed, I think this was causing me issues but it was not something I would have guessed was even related.
Thanks for all your help!