Properly reading bytes from serial monitor

Using Arduino Programming Questions
#1

Hello everyone!

I am having a weird issue where I keep on getting a “ten” into one of my arrays (array of two lol).
Despite the previous similar code working flawlessly. (Or at least I assume it is because it works as expected).

I simply want to input through the Serial Monitor a 13 bit address in hex. However, the “higher” 5 bits are giving me an issue.

Here is my code with a comment showing where I think the problem is
(lines 42-64 and 203-223)

Also I am using an Arduino Nano from Flavin but the one I am using, contrary to a lot of the comments
on Amazon, does not need the Old Bootloader.

Thanks for any help! Even if it means a far simpler way (or simply more effective) of implementing my goal.

#include<String.h>
#include<Hardwareserial.h>

#define      R_CLK      2
#define      SR_CLK     3
#define      W_CLK      4
#define      D_CLK      5
#define      SER_OUT    6
#define      SER_IN     7
#define      LOAD_LOW   8

const byte   DEFAULT_DATA = 0xff;

const byte   WRITE = 0x00;
const byte   READ  = 0x80;

byte         mode_bit;

byte         so_data;
byte         addr_hi;
byte         addr_low;

byte         data_out[3];
byte         data_in;


const String MODE_READ  = "read";
const String MODE_WRITE = "write";

String       user_mode;


//Setup user input data: address and write input
byte setup_user_input_data(byte addr_low_buff[], byte addr_hi_buff[]) {

  bool low_error = true;
  bool hi_error  = true;

  byte error_code = 0x00;

  //Verify user input is valid
  for (int i = 0; i < 2; i++) {
    if (addr_low_buff[i] > 96 and addr_low_buff[i] < 103) {
      low_error = false;
    }
    else if (low_error) {
      if (addr_low_buff[i] > 47 and addr_low_buff[i] < 58) {
        low_error = false;
      }
    }

    //Problem with code ere as well (52-64)
    if (addr_hi_buff[0] > 96 and addr_hi_buff[0] < 103) {
      hi_error = false;
    }
    else if (low_error) {
      if (addr_hi_buff[0] > 47 and addr_hi_buff[0] < 58) {
        hi_error = false;
      }
    }

    if (addr_hi_buff[1] > 47 and addr_hi_buff[1] < 50) {
      hi_error = false;
    }

    error_code = (hi_error * 2) + low_error;
    if (error_code) {
      return error_code;
    }

    //Interpret user data
    for (int i = 0; i < 2; i++) {
      if (addr_low_buff[i] > 96 and addr_low_buff[i] < 103) {
        addr_low_buff[i] -= 87;
      }
      else if (addr_low_buff[i] > 47 and addr_low_buff[i] < 58) {
        addr_low_buff[i] -= 48;
      }
    }

    if (addr_hi_buff[0] > 96 and addr_hi_buff[0] < 103) {
      addr_hi_buff[0] -= 87;
    }
    else if (addr_hi_buff[0] > 47 and addr_hi_buff[0] < 58) {
      addr_hi_buff[0] -= 48;
    }
  }

  if (addr_hi_buff[1] > 47 and addr_hi_buff[1] < 50) {
    addr_hi_buff[1] -= 48;
  }

  addr_low = (addr_low_buff[1] << 4) + addr_low_buff[0];
  addr_hi  = (addr_hi_buff[1] << 4)  + addr_hi_buff[0];

  return error_code;
}


//Setup data_out function
//May need to need change  "bit mask" based on zero for EEPROM, that is, an erased EEPROM is 0xff.
void data_out_setup() {

  switch (mode_bit) {
    case WRITE:

      addr_hi    = addr_hi and mode_bit;

      data_out[0] = 0x00 or addr_low;
      data_out[1] = 0x00 or addr_hi;
      data_out[2] = 0x00 or so_data;

      break;
    case READ:

      addr_hi    = addr_hi and mode_bit;

      data_out[0] = 0x00 or addr_low;
      data_out[1] = 0x00 or addr_hi;
      data_out[2] = 0x00 or DEFAULT_DATA;
  }
}

//Read function
void read_EEPROM() {

  mode_bit = READ;
  data_out_setup();

  shiftOut(SER_OUT, SR_CLK, MSBFIRST, data_out[2]);
  shiftOut(SER_OUT, SR_CLK, MSBFIRST, data_out[1]);
  shiftOut(SER_OUT, SR_CLK, MSBFIRST, data_out[0]);

  digitalWrite(R_CLK, HIGH);
  delay(50);
  digitalWrite(R_CLK, LOW);

  digitalWrite(LOAD_LOW, LOW);
  delay(50);
  digitalWrite(LOAD_LOW, HIGH);

  data_in = shiftIn(SER_IN, D_CLK, MSBFIRST);
}

//Write function
void write_EEPROM() {

  mode_bit = WRITE;
  data_out_setup();

  shiftOut(SER_OUT, SR_CLK, MSBFIRST, data_out[2]);
  shiftOut(SER_OUT, SR_CLK, MSBFIRST, data_out[1]);
  shiftOut(SER_OUT, SR_CLK, MSBFIRST, data_out[0]);

  digitalWrite(R_CLK, HIGH);
  delay(50);
  digitalWrite(R_CLK, LOW);

  digitalWrite(LOAD_LOW, LOW);
  delay(50);
  digitalWrite(LOAD_LOW, HIGH);
}

void setup() {

  pinMode(R_CLK, OUTPUT);
  pinMode(SR_CLK, OUTPUT);
  pinMode(W_CLK, OUTPUT);
  pinMode(SER_OUT, OUTPUT);
  pinMode(SER_IN, INPUT);
  pinMode(LOAD_LOW, OUTPUT);

  digitalWrite(R_CLK, LOW);
  digitalWrite(SR_CLK, LOW);
  digitalWrite(W_CLK, LOW);
  digitalWrite(SER_OUT, LOW);
  digitalWrite(LOAD_LOW, HIGH);
}

void loop() {

  byte addr_low_buff[2] = {0x00, 0x00};
  byte addr_hi_buff[2]  = {0x00, 0x00};
  byte so_data_buff[2]  = {0x00, 0x00};

  byte error_code;

  Serial.begin(9600);

  Serial.print("Enter mode read or write: ");

  while (Serial.available() == 0) {}
  user_mode = Serial.readString();

  Serial.print('\n');
  Serial.print("Lower eight bits of address in hex: ");

  while (Serial.available() == 0) {}
  addr_low_buff[1] = Serial.read();
  while (Serial.available() == 0) {}
  addr_low_buff[0] = Serial.read();

  //Problem with code here (203-223)
  Serial.print('\n');
  Serial.print("Higher five bits of address in hex: ");

  while ((Serial.available() == 0)) {}
  addr_hi_buff[1] = Serial.read();
  while (Serial.available() == 0) {}
  addr_hi_buff[0] = Serial.read();

  //Debugging code right here
  Serial.print('\n');
  Serial.print("addr_low_buff: ");
  Serial.print(addr_low_buff[1]);
  Serial.print('_');
  Serial.print(addr_low_buff[0]);

  Serial.print('\n');
  Serial.print("addr_hi_buff: ");
  Serial.print(addr_hi_buff[1]);
  Serial.print('_');
  Serial.print(addr_hi_buff[0]);

  error_code = setup_user_input_data(addr_low_buff, addr_hi_buff);

  switch (error_code) {
    case 0:
      break;
    case 1: Serial.print('\n');
      Serial.print("User input invalid on lower address input. Program ending.");
      do {} while (true);
      break;
    case 2: Serial.print('\n');
      Serial.print("User input invalid on higer address input. Program ending.");
      do {} while (true);
      break;
    case 3: Serial.print('\n');
      Serial.print("User input invalid on both address inputs. Program ending.");
      do {} while (true);
  }

  if (user_mode == MODE_WRITE) {

    Serial.print('\n');
    Serial.print("Enter data to write to EEPROM in hex: ");

    while (Serial.available() == 0); {}
    so_data_buff[1] = Serial.read();
    while (Serial.available() == 0); {}
    so_data_buff[0] = Serial.read();

    so_data = ((so_data_buff[1] - 87) << 4) + (so_data_buff[0] - 48);

    Serial.print('\n');
    Serial.print('\n');

    write_EEPROM();
  }
  else if (user_mode == MODE_READ) {

    read_EEPROM();

    Serial.print('\n');
    Serial.print("Data read in hex form: 0x");
    Serial.print(data_in, HEX);
    Serial.print('\n');
    Serial.print('\n');
  }

}
#2

What input are you sending to the arduino and what does the arduino output for this specific input?
In any case:

addr_hi    = addr_hi and mode_bit;	
data_out[0] = 0x00 or addr_low;
data_out[1] = 0x00 or addr_hi;
data_out[2] = 0x00 or so_data;

I don't think this does what you want it to do. 'and' and 'or' are not binary operators. Also or'ing something with zero is useless.

#3

This is mostly a rough draft for something bigger.

I commented that I would change it too. EEPROMs are all ones when "erased" so the reason I have
it or'd right now is for user input (keeping the code I want to use in the future (and instead of or)). It will be easier for me when testing my circuit if "zero" is actually zero.

But this post is not about that part of the code anyways. Unless there is something actually wrong that I am unaware of, please try to focus on the known problem!

#4

That code seems very cumbersome, but your immediate problem is that you have the serial monitor set to send a newline character, which is a decimal 10 in ascii, and after reading in the low address, you are leaving the newline in the input buffer. When you then read in the high address, you are reading an ascii newline into addr_high_buff[1].

  Serial.print("Lower eight bits of address in hex: ");

  while (Serial.available() == 0) {}
  addr_low_buff[1] = Serial.read();
  while (Serial.available() == 0) {}
  addr_low_buff[0] = Serial.read();
  while (Serial.available() == 0) {}  //need to wait for the /n from input then get rid of it
  Serial.read();

  //Problem with code here (203-223)
  Serial.print('\n');
  Serial.print("Higher five bits of address in hex: ");

  while ((Serial.available() == 0)) {}
  addr_hi_buff[1] = Serial.read();
  while (Serial.available() == 0) {}
  addr_hi_buff[0] = Serial.read();
  while (Serial.available() == 0) {}  //need to wait for the /n from input then get rid of it
  Serial.read();
#5

HanzoSheana:
This is mostly a rough draft for something bigger.

How would I know? This is the only information you have supplied to get help in this forum.

HanzoSheana:
I commented that I would change it too.

Comments don't mean anything. In most cases they don't even relate to the code.

HanzoSheana:
But this post is not about that part of the code anyways. Unless there is something actually wrong that I am unaware of, [...]

Yes, there is indeed something very wrong with this.

addr_hi = addr_hi and bit_mode;

// is the same as
addr_hi = addr_hi && bit_mode;

HanzoSheana:
[...] please try to focus on the known problem!

That is what I tried to do, so I asked "What input are you sending to the arduino and what does the arduino output for this specific input?" in my last post.

#6

It is not a good idea to use the String (capital S) class on an Arduino as it can cause memory corruption in the small memory on an Arduino. This can happen after the program has been running perfectly for some time. Just use cstrings - char arrays terminated with ‘\0’ (NULL).

Have a look at the examples in Serial Input Basics - simple reliable ways to receive data. There is also a parse example to illustrate how to extract numbers from the received text.

…R

#7