Cannot display a chr string using Wire.read() and u8g2.drawStr()

Question

I'm still getting myself acquainted with C++, so I expect this is a very basic issue I'm facing. I've got an OLED screen connected to a Teensy LC and can display text on it just fine. The Teensy receives bytes via I2C, and I want to reassemble those bytes into strings and display them on the OLED.

My relevant code:

#define I2C_ADDRESS 4
char buf[16];

void setup(void) {
  Serial.begin(9600);
  Wire.begin(I2C_ADDRESS);
  Wire.onReceive(receiveData);
  u8g2.begin();
}
void loop(void) {
  // U8G2 setup code snipped
  // This line doesn't output anything on the screen:
  u8g2.drawStr(0, 0, buf);
  // This line does:
  u8g2.drawStr(0, 20, "Foobar!");
  u8g2.sendBuffer();
  delay(1000);
}

void receiveData(int byteCount){
  Serial.print("Data received: ");
  int i;
  for (i=0; i<byteCount; i++)
  {
    buf[i] = Wire.read();
    Serial.println(buf[i]);
  }
  buf[i] = '\0';
  Serial.println(buf);
}

In the receiveData() function, the Serial.println() within the for loop prints out each letter of the payload as it's received, one after the other - but the Serial.println() below the for loop, which in my estimation should print out the whole message, prints out a blank line. I presume that's related to the fact that the received string doesn't show up on the OLED display.

Answer 1

This is kind of convoluted to put as a comment and it is sort of an answer, so I'm making it an answer. Just to warn you, I have not tried to compile any of the code snippets here.

Wire.onReceive(receiveData); registers your function so that it can be later called back in the context of the interrupt service routine used for handling I2C ("Wire") data. Or at least this is how it usually works. So when your receiveData function is running you are in the middle of handling an interrupt associated with the I2C peripheral.

Using Serial.print(), .println(), .write(), and many other things, including non-serial related things like delay() inside of an interrupt service is usually a no-no. On the AVR Arduino core, if the outbound serial buffer happens to fill, your code will deadlock with the underlying .write() waiting for there to be room in the outbound serial buffer and the serial transmit interrupt waiting for .write() to get done waiting for it. I don't know how the Teensy LC and its MCU are configured, whether or not it prioritizes interrupts, etc, but it is likely that you're still not supposed to call functions like these inside the interrupt context.

There's really not a lot you're supposed to do in an ISR except:

• communicate with hardware (which is being done for you here)
• manage the non-volatile variables that belong to the ISR itself.
• set global volatile qualified variables to communicate with the main line of execution.
• call functions that have no shared state with the main line of execution.
• call functions that have shared state but are carefully designed to be called from both ISR contexts and main line of execution.

That said, I don't know precisely why you're seeing what you're seeing, but I'm not overly surprised either. The above may be related to it, but I haven't drawn that connection yet.

The way you currently have receiveData, it may overflow buf. All that's required for that is for the sending side to send more than 15 bytes. Any number of weird things can happen if you manage to overflow. Is it related? Maybe.

For communicating between the receiveData running in the ISR context and the main line of execution in loop, buf should be volatile qualified. Without that the main line of execution can behave as though buf never changes after it is first seen. Because of how you're using it and familiarity with the compiler, I'm going to guess that this isn't manifesting as a bug, but I don't know that; it could be related.

So I'd recommend updating something like your buf declaration to volatile qualified and removing the Serial print calls from the receiveData function, and preventing overflow:

volatile char buf[16];

/// ...

void receiveData(int byteCount){
  static const size_t room_for_non_null_chars = sizeof buf - 1;
  const size_t read_amount = min(byteCount, room_for_non_null_chars);

  int i;
  for (i = 0; i < read_amount; ++i) {
    buf[i] = Wire.read();
  }

  buf[i] = '\0';
}

If it weren't for the fact that we need to read into something volatile qualified the above loop could really just be a read call taking the buffer and size. You can see that's updated to prevent overflow.

Part of loop would then become something like:

void loop() {
  char non_volatile_buf[sizeof buf];
  size_t write_index = 0;

  noInterrupts();
  for (char c: buf) {
    non_volatile_buf[write_index++] = c;
  }
  interrupts();

  // U8G2 setup code snipped
  // This line doesn't output anything on the screen:
  u8g2.drawStr(0, 0, non_volatile_buf);

  // ...

We're turning off interrupts while copying the data into a local buffer so that the onReceive handler cannot interrupt the use of the buf, updating while you're in the middle of using it in drawStr.

If you care about losing messages there's more that would need to be done. Surrounding what to do if you've got a call to receiveData while there's a message currently in buf. The way the above is written it just drops whatever is in there whether it's been seen by loop() or not.

Answer 2

I didn't get a load of sleep last night but I finally figured it out, thanks to timemage's post. I'm concerned that my solution is hyper-specific, but I'd rather share it to save someone the trouble in future.

1. timemage was right in that the buffer should really be volatile qualified and that as little processing as possible should happen in what I now understand is an ISR. I couldn't get their code to work, but I eventually stumbled on i2c_t3, an enhanced I2C library specifically for Teensy 3.0/3.1/LC. Looking at the library's code and especially its basic peripheral example, it looks like it implements similar techniques to those timemage recommended.
2. The other thing I didn't consider was the way the controller was communicating. I was using the smsbus2 Python library, and at one point I changed the address offset from 0x00 to 0x01. This mysteriously seemed to make the Teensy code work, but I don't know why!

data = [1, 2, 3, 4, 5, 6, 7, 8]
bus.write_i2c_block_data(0x21, 0x01, data)

3. Lastly, from reading forum posts about saving data to variables from I2C, it looks like a lot of quite simple code that does work on other Arduino devices doesn't work on the Teensy.¹ I'd have to get an Arduino device to test this theory, though.

Because I'm a bad scientist, I haven't checked whether any of my original code or timemage's improved code works when I offset the address in the smbus2 code. But for what it's worth, these are the things I did to solve my problem!

---

1. See for example: https://forum.arduino.cc/index.php?topic=570597.0; https://forum.arduino.cc/index.php?topic=619902.0.