Problem with direct port manipulation and bitmask

Hello,
I am implementing a GPIB-USB interface using an Arduino Uno and I need to speed it up, so I thought to discard all digitalWrite, digitalRead and pinMode functions and use the direct port manipulation. Yet, I had a problem when I have to set the data pins of the GPIB.

This is the code of the first implementation with digitalWrite

  digitalWrite(2, bitRead(~x, 0));
  digitalWrite(3, bitRead(~x, 1));
  digitalWrite(4, bitRead(~x, 2));
  digitalWrite(5, bitRead(~x, 3));
  digitalWrite(6, bitRead(~x, 4));
  digitalWrite(7, bitRead(~x, 5));
  digitalWrite(8, bitRead(~x, 6));
  digitalWrite(9, bitRead(~x, 7));

So i need to manipulate PORTB (pins 8,9) and PORTD (pins 2,7). Here is the code i'm trying.

  a = ~x;
  y = a & B11000000;
  y = y >> 6;
  PORTB = PORTB | y;
  z = x << 2;
  z = z & B11111100;
  PORTD = PORTD | z;

It should work logically, but when i put into my code it doesn't. x is the byte i want to use to set PORTD and PORTB. I already set the proper input/output with the DDR registers.

Any suggestion?

Thanks in advance,

Dante

little error in the code... sorry, this is the right one:

  x = ~x;
  y = x & B11000000;
  y = y >> 6;
  PORTB = PORTB | y;
  z = x << 2;
  z = z & B11111100;
  PORTD = PORTD | z;

If it's not working, you are better off showing all the code, so people can work out what is not working, rather than posting snippets of some of your code.

Sorry, here it is all the code. When I use the digitalWrite function in the set_dio() function it works perfectly, when I change it with the direct manipulation it stops working...it doesnt address the instrument properly.

/*
 * GIPB USB Adapter
*/

#include <string.h>
#include <avr/io.h>
#include <avr/wdt.h>

// Define PIN name
#define DIO1  2
#define DIO2  3
#define DIO3  4
#define DIO4  5
#define DIO5  6
#define DIO6  7
#define DIO7  8
#define DIO8  9
#define IFC   A5
#define EOI   A4
#define DAV   A3
#define NRFD  A2
#define NDAC  A1
#define ATN   A0
#define REN   GND
#define SRQ   10


// set Commando val
char com[256] = "";
char c = 0;
int p = 0;
unsigned long t;
byte pin;

void setup() {
  
  // initialize the serial device
  Serial.begin(9600);
  
  // initialize gpib line
  pinMode(EOI, OUTPUT); digitalWrite(EOI, HIGH);
  pinMode(DAV, OUTPUT); digitalWrite(DAV, HIGH); 
  pinMode(NDAC, OUTPUT); digitalWrite(NDAC, LOW);
  pinMode(NRFD, OUTPUT); digitalWrite(NRFD, LOW);

  //activate the watch dog with time_out = 2 s
  wdt_enable(WDTO_4S);
  
  Serial.println("ELI 1.2 ready!");

}

void loop() {
  int i;
    
  if (Serial.available()) {
        c = Serial.read();
        *(com + p) = c; *(com + p + 1) = 0; 
        p++;
      }
      if (';' == c) {
        *(com + p - 1) = 0;
        if (0 == strcmp("IFC", com)) { gpibIFC();
        } else if (0 == strcmp("DCL", com)) { gpibDCL();
        } else if (0 == strcmp("STA", com)) { gpibLineStatus();
        } else if (0 == strcmp("RES", com)) {gpibRestart();
        } else {
          char command;
          int addr;
          char option[255];
          sscanf(com, "%c%2d", &command, &addr);
          for (i = 4 ; i < strlen(com) ; i++) {
            option[i-4] = *(com + i);
          } option[i-4] = 0;
          strcat(option, "\r\n");
          
          if ('W' == command) {  
            t = micros();
            gpibTalk((byte)addr, option);
            t = micros() - t;
            Serial.println("time elapsed: ");
            Serial.println(t);
          } else if ('R' == command) {
            char str[255] = "";
            gpibListen((byte)addr, str, "\r\n"); Serial.println(str);
          } else if ('C' == command) {
            gpibSDC((byte)addr);
          } 
        }
        *com = 0; p = 0; c = 0;
      }
      
      wdt_reset();
      
}

void gpibLineStatus(void) {
  pinMode(ATN, INPUT); digitalWrite(ATN, HIGH); Serial.print(" ATN="); Serial.print(digitalRead(ATN));
  pinMode(DAV, INPUT); digitalWrite(DAV, HIGH); Serial.print(", DAV="); Serial.print(digitalRead(DAV));
  pinMode(NRFD, INPUT); digitalWrite(NRFD, HIGH); Serial.print(", NRFD="); Serial.print(digitalRead(NRFD));
  pinMode(NDAC, INPUT); digitalWrite(NDAC, HIGH); Serial.print(", NDAC="); Serial.print(digitalRead(NDAC));
  pinMode(EOI, INPUT); digitalWrite(EOI, HIGH); Serial.print(", EOI="); Serial.print(digitalRead(EOI));
  Serial.println(", DIO8-1=");
  pinMode(DIO8, INPUT); digitalWrite(DIO8, HIGH); Serial.print(digitalRead(DIO8));
  pinMode(DIO7, INPUT); digitalWrite(DIO7, HIGH); Serial.print(digitalRead(DIO7));
  pinMode(DIO6, INPUT); digitalWrite(DIO6, HIGH); Serial.print(digitalRead(DIO6));
  pinMode(DIO5, INPUT); digitalWrite(DIO5, HIGH); Serial.print(digitalRead(DIO5));
  pinMode(DIO4, INPUT); digitalWrite(DIO4, HIGH); Serial.print(digitalRead(DIO4));
  pinMode(DIO3, INPUT); digitalWrite(DIO3, HIGH); Serial.print(digitalRead(DIO3));
  pinMode(DIO2, INPUT); digitalWrite(DIO2, HIGH); Serial.print(digitalRead(DIO2));
  pinMode(DIO1, INPUT); digitalWrite(DIO1, HIGH); Serial.println(digitalRead(DIO1));

}

byte get_dio() {
  byte x = 0;
  
  pinMode(DIO1, INPUT); bitWrite(x, 0, !digitalRead(DIO1));
  pinMode(DIO2, INPUT); bitWrite(x, 1, !digitalRead(DIO2));
  pinMode(DIO3, INPUT); bitWrite(x, 2, !digitalRead(DIO3));
  pinMode(DIO4, INPUT); bitWrite(x, 3, !digitalRead(DIO4));
  pinMode(DIO5, INPUT); bitWrite(x, 4, !digitalRead(DIO5));
  pinMode(DIO6, INPUT); bitWrite(x, 5, !digitalRead(DIO6));
  pinMode(DIO7, INPUT); bitWrite(x, 6, !digitalRead(DIO7));
  pinMode(DIO8, INPUT); bitWrite(x, 7, !digitalRead(DIO8));
  
  return x;
}

void set_dio(byte x) {
  
  byte a = x;
  byte y;
  byte z;
  // SET DIO 1-8 OUTPUT
  DDRD = DDRD | B11111100;
  DDRB = DDRB | B00000011;
  
  
  x = ~x;
  y = x & B11000000;
  y = y >> 6;
  PORTB = PORTB | y;
  z = x << 2;
  z = z & B11111100;
  PORTD = PORTD | z;
  
  /*
  digitalWrite(DIO1, bitRead(~x, 0));
  digitalWrite(DIO2, bitRead(~x, 1));
  digitalWrite(DIO3, bitRead(~x, 2));
  digitalWrite(DIO4, bitRead(~x, 3));
  digitalWrite(DIO5, bitRead(~x, 4));
  digitalWrite(DIO6, bitRead(~x, 5));
  digitalWrite(DIO7, bitRead(~x, 6));
  digitalWrite(DIO8, bitRead(~x, 7));
  */
}

void gpibWrite(byte data) {
  
  // set NRFD and NDAC input, and DAV output
  DDRC = DDRC | B00001000;
  DDRC = DDRC & B11111001;
  
  // wait until (LOW == NDAC)
  while (B11111111 == (PORTC | B11111101)) { ; } 
  
  // output data to DIO
  set_dio(data);
  
  // wait until (HIGH == NRFD)
  while (B11111011 == (PORTC & B11111011)) { ; }
  
  // validate data
  PORTC = PORTC & B11110111;  //set DAV LOW
  
  // wait until (HIGH == NDAC)
  while (B11111101 == (PORTC & B11111101)) { ; }
  
  PORTC = PORTC | B00001000;  // set DAV HIGH
  set_dio(0); 
    
  return;
}

boolean gpibRead(byte *data) {
  
  boolean ret = false;
  
  // prepare to listen
  pinMode(NRFD, OUTPUT);
  digitalWrite(NRFD, HIGH);
  
  // wait until (LOW == DAV)
  pinMode(DAV, INPUT);
  while (HIGH == digitalRead(DAV)) { ; }
  
  // Ready for data
  digitalWrite(NRFD, LOW);
  
  // read from DIO
  *data = get_dio();
  
  // check EOI
  pinMode(EOI, INPUT);
  if (LOW == digitalRead(EOI)) {
    ret = true;
  } else {
    ret = false;
  }
  
  // data accepted
  pinMode(NDAC, OUTPUT);
  digitalWrite(NDAC, HIGH);
  
  // wait until invalid data
  while (LOW == digitalRead(DAV)) { ; }
  digitalWrite(NDAC, LOW);
  
  return ret; // return true when EOI==LOW
}

void gpibTalk(byte addr, char *str) {
  
  // attention
  DDRC = DDRC | B00010001; // set EOI and ATN output
  PORTC = PORTC | B00010000; // set EOI HIGH
  PORTC = PORTC & B11111110; // set ATN LOW
  
  // unlisten
  gpibWrite(0x3F);
  
  // talker address
  gpibWrite(0x40); 
  
  // listener address
  gpibWrite((byte)(0x20 + addr)); 
  
  // end of attention
  PORTC = PORTC | B00000001; // set ATN HIGH
    
  // write string
  while (0 != *(str + 1)) {
    gpibWrite(*str);
    str++;
  }
  
  // write last char
  PORTC = PORTC & B11101111; // set EOI LOW
  gpibWrite(*str);
  PORTC = PORTC | B00010000; // set EOI HIGH  
}

boolean gpibListen(byte addr, char *str, char* del) {
  // attention
  pinMode(ATN, OUTPUT);
  digitalWrite(ATN, LOW);
  
  // unlisten
  gpibWrite(0x3F);
  
  // talker address
  gpibWrite((byte)(0x40 + addr));
  
  // listener address
  gpibWrite(0x20); 
  
  // end of attention
  pinMode(NRFD, OUTPUT); digitalWrite(NRFD, LOW);
  pinMode(NDAC, OUTPUT); digitalWrite(NDAC, LOW);
 
  digitalWrite(ATN, HIGH); 
  
  // recieve data
  int i, s, len = strlen(del);
  byte c;
  boolean isLast, isDel;
  
  s = 0;
  do {
    isLast = gpibRead(&c); *(str) = c;
    isDel = true;
    for (i = 0 ; i < len ; i++) {
      isDel = isDel & (*(str - i) == *(del + len - i - 1));
    }
    str++;
  } while (!(isLast || isDel));
  *(str) = 0;

  return true;
}

void gpibIFC(void) {
  pinMode(IFC, OUTPUT); digitalWrite(IFC,LOW); delayMicroseconds(128);
  digitalWrite(IFC, HIGH);
}

void gpibDCL(void) {
  // attention
  pinMode(ATN, OUTPUT); digitalWrite(ATN, LOW);
  
  // send DCL
  gpibWrite(0x14);
  
  // end of attention
  digitalWrite(ATN, HIGH); 
}

void gpibSDC(byte addr) {
  // attention
  pinMode(ATN, OUTPUT); digitalWrite(ATN, LOW); 
  
  // unlisten
  gpibWrite(0x3F);
  
  // send ADDRESS for device clear
  gpibWrite((byte)(32 + addr)); 
  
  // send SDC
  gpibWrite(0x04);
  
  // end of attention
  digitalWrite(ATN, HIGH); 
}

void gpibRestart() {
  Serial.flush();
  pinMode(EOI, OUTPUT); digitalWrite(EOI, HIGH);
  pinMode(DAV, OUTPUT); digitalWrite(DAV, HIGH); 
  pinMode(NDAC, OUTPUT); digitalWrite(NDAC, LOW);
  pinMode(NRFD, OUTPUT); digitalWrite(NRFD, LOW);
  pinMode(ATN, OUTPUT); digitalWrite(ATN, HIGH);

}

Found the problem. I wasn't initializing the PORT registers, so the were dirty from operations I did before...i just put a PORT = B00000000 before everytime I wanted to set them and it started working again :smiley:

kwsafa:
Found the problem. I wasn't initializing the PORT registers, so the were dirty from operations I did before...i just put a PORT = B00000000 before everytime I wanted to set them and it started working again :smiley:

Well done for figuring it out, and thank you for letting us know.

kwsafa:
Found the problem. I wasn't initializing the PORT registers, so the were dirty from operations I did before...i just put a PORT = B00000000 before everytime I wanted to set them and it started working again :smiley:

That will output glitches on the pins - you set all pins LOW, then set some HIGH. Next time you set them
all LOW again. If the other pins matter (such as pin D1, the TX pin), this isn't advisable.

The normal idiom is

  x = ~x;
  PORTB = (PORTB & 0xFC) | (x >> 6) ;
  PORTD = (PORTD & 0x03) | (x << 2) ;

No need to mask as the shifts will feed in zeroes in both cases (assuming x is either unsigned or more than
8 bits). PORT registers are live - anything you write to them immediately is driven to the pins if they
are outputs. They also retain internal state (what you last wrote) irrespective of whether the pins are
being driven.

Note that if you are using interrupt routines you have to be careful as the line

  PORTB = (PORTB & mask) | value ;

Is not and cannot be atomic. There are ways to flip a single bit in a PORT register that are
atomic, but in general you have to mask interrupts around direct port access if you have
interrupt routines writing to the same port. See how digitalWrite() does it for full details.

There is a sneaky way to toggle pins by writing to the PIN register - any ones written toggle
the relevant pins (this is atomic). In fact I think you can do

  x = ~x;
  PINB = (PORTB ^ (x >> 6)) & 0x03 ;
  PIND = (PORTD ^ (x << 2)) & 0xFC ;

(The combination of reading PORT before writing PIN is not atomic however, so this is rather
obfuscated, however if you are toggling a clock pin its handy to know about writing PIN registers)

Thanks a lot for all the tricks, it was my first time doing this and i did not consider atomic/not atomic functions at all. Luckily my program had no problems but it's good to know it for next projects.