Arduino can't "AND" large value correctly (was Playstation controller to Arduin)

I am trying to set up a test to connect the PSX (older digital only version) to Arduino and have it turn on LEDs depending on what is pressed. Part of it is working, directional and start button. The 4 action buttons are not doing anything.

I used serial.println to read the data, the buttons on controller are working and is being read so I know it is not a problem with the controller. But for some reason the 4 buttons are not turning on the LEDs. The number value for the 4 buttons are correct yet I am wondering if I missed something somewhere.

Here's the code:

/*  PSX Controller Decoder Library (Psx.pde)
 	Written by: Kevin Ahrendt June 22nd, 2008
 	
 	Controller protocol implemented using Andrew J McCubbin's analysis.
 	http://www.gamesx.com/controldata/psxcont/psxcont.htm
 	
 	Shift command is based on tutorial examples for ShiftIn and ShiftOut
 	functions both written by Carlyn Maw and Tom Igoe
 	http://www.arduino.cc/en/Tutorial/ShiftIn
 	http://www.arduino.cc/en/Tutorial/ShiftOut
 
 This program is free software: you can redistribute it and/or modify
 it under the terms of the GNU General Public License as published by
 the Free Software Foundation, either version 3 of the License, or
 (at your option) any later version.
 
 This program is distributed in the hope that it will be useful,
 but WITHOUT ANY WARRANTY; without even the implied warranty of
 MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 GNU General Public License for more details.
 
 You should have received a copy of the GNU General Public License
 along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */

#include <Psx.h>                                          // Includes the Psx Library 

/* controller pinouts are:
 1  2  3   4  5  6   7  8  9
 -------------------------------
 | o  o  o | o  o  o | o  o  o |  (at the Controller)
 \_____________________________/
 
 1 DATA     5 Vcc
 2 CMD      6 ATT
 3 motor V+ 7 CLK
 4 ground   9 ACK
 
 3 motor v+ (rumble) is not used, 8 has no connection, and 9 ACK is not needed.  5 Vcc is intended to be 3.3v but 5v will be fine
 */

#define dataPin 2
#define cmndPin 3
#define attPin 4
#define clockPin 5
#define Up 0x0008
#define Down 0x0002
#define Left 0x0001
#define Right 0x0004
#define PsxSqr 0x0100
#define PsxTri 0x0800
#define PsxX 0x0200
#define PsxCrl 0x0400
#define Start 0x0010

/* not implemented yet
 #define psxStrt		0x0010
 #define psxR1		0x1000
 #define psxL1		0x2000
 #define psxR2		0x4000
 #define psxL2		0x8000
 */


Psx Psx;                                                  // Initializes the library

unsigned int data = 0;                                    // data stores the controller response

void setup()
{
  Serial.begin(9600);  // for monitoring within arduino, disable if not needed
  Psx.setupPins(dataPin, cmndPin, attPin, clockPin, 10);  // Defines what each pin is used
  // (Data Pin #, Cmnd Pin #, Att Pin #, Clk Pin #, Delay)
  // Delay measures how long the clock remains at each state,
  // measured in microseconds. 10 is fine.

  pinMode(A0, OUTPUT);                                    //not enough digital out, using analog out as digital
                                                 //can set output for A1-A5 to handle Select, L+R buttons.
  pinMode(6,  OUTPUT);
  pinMode(7,  OUTPUT);
  pinMode(8,  OUTPUT);
  pinMode(9,  OUTPUT);
  pinMode(10, OUTPUT);
  pinMode(11, OUTPUT);
  pinMode(12, OUTPUT);                                                      
  pinMode(13, OUTPUT);                                    // Establishes all 8 digital Pin as an output so the LED can be seen
}

void loop()
{
  data = Psx.read();                                      // Psx.read() initiates the PSX controller and returns
                                                          // the button data

  {
    digitalWrite(6,  data & Up );      
    digitalWrite(7,  data & Down );
    digitalWrite(8,  data & Left );
    digitalWrite(9,  data & Right );
    digitalWrite(10, data & PsxSqr );
    digitalWrite(11, data & PsxTri );
    digitalWrite(12, data & PsxX );                    
    digitalWrite(13, data & PsxCrl );
    digitalWrite(A0, data & Start );

//next 3 lines are for monitoring with arduino, to check if it's working or not
    if (data>0) {  
      Serial.println(data);
    }                                                        
  }
}

The wiring are as:
Playstation Vcc and Ground are connected to common v and g, the 4 lines (described in the code box above) from controller to digital pin 2, 3, 4, and 5. Digital pins 6-13 and analog pin A0 are set as output, goes to anode lead of a LED. Cathode lead goes through a resistor to ground.

Forgot to add the library, it is original version

PSX library.zip (15.4 KB)

^^ bump ^^

Back to the front it goes...

Surely someone can look at the code and tell me if it is OK and the problem is elsewhere or something?

What does your serial print tell you when the buttons are pressed?

Can you put some test code in setup that lights all the leds?

wildbill:
What does your serial print tell you when the buttons are pressed?

Can you put some test code in setup that lights all the leds?

When I do a straight variation of basic blink, using all pins, all LED comes on and off as expected. LED wiring isn't the problem and the chip isn't fried for sure. I did find something that may explain my problem:

I tried something different, using SNES pad instead: Google Code Archive - Long-term storage for Google Code Project Hosting.

I changed the sketch to match the original wiring for LED output.

#include <SNESpad.h>

// put your own strobe/clock/data pin numbers here -- see the pinout in readme.txt
SNESpad nintendo = SNESpad(3,2,4);

int state = 0;

void setup() {
//    Serial.begin(9600);   
  pinMode(6,  OUTPUT);
  pinMode(7,  OUTPUT);
  pinMode(8,  OUTPUT);
  pinMode(9,  OUTPUT);
  pinMode(10, OUTPUT);
  pinMode(11, OUTPUT);
  pinMode(12, OUTPUT);    
  pinMode(13, OUTPUT);
  pinMode(A0,  OUTPUT);
  pinMode(A1, OUTPUT);
}

void loop() {
  
  state = nintendo.buttons();

  digitalWrite(A0,  state & SNES_A );     
  digitalWrite(12,  state & SNES_B );      
  // digitalWrite(xx,  state & SNES_SELECT );
  digitalWrite(A1,  state & SNES_START );
  digitalWrite(6,  state & SNES_UP );
  digitalWrite(7, state & SNES_DOWN );
  digitalWrite(8, state & SNES_LEFT );
  digitalWrite(9, state & SNES_RIGHT );
  digitalWrite(10, state & SNES_Y );
  digitalWrite(11, state & SNES_X);

  delay(1);
//    Serial.println(state); 

}

Using serial print, I am able to get a reading from the SNES controller like the PSX pad. However the LED for button X, A, L and R would not work. Coincidentally, these 4 buttons also had hex value of 0x0100 or higher, which happens to be the same problem with PSX controller, buttons whose value is 0x0100 or higher is not working.

Looks to me that Ardiuno isn't doing the "and" part right with value higher than a byte.

Which means I need suggestion on correcting the math where value is higher than one byte (higher than FF) :

digitalWrite(xx state & pin) needs to be changed to what?

What you are missing is int's are 16-bits under AVR processors like Arduino. I suspect the playstation system had ints as 32-bits. I would suggest changing types to long/unsigned long at the least, or better: int32_t/uint32_t which are types defined by the includes that the IDE automatically includes that say you need at least 32-bits.

Just for good measure, I would add a 'L' suffix to the #defines. Since the IDE uses the C++ language underneath, you could change the #define to be const uint32_t values (const ints do not allocate storage under C++ like they do for C).

#include <Psx.h>       
#define dataPin 2
#define cmndPin 3
#define attPin 4
#define clockPin 5
#define Up 0x0008LU
#define Down 0x0002LU
#define Left 0x0001LU
#define Right 0x0004LU
#define PsxSqr 0x0100LU
#define PsxTri 0x0800LU
#define PsxX 0x0200LU
#define PsxCrl 0x0400LU
#define Start 0x0010LU

Psx Psx;                                                  
long unsigned int data = 0;         
                        
void setup()
{
  // Serial.begin (9600);
  Psx.setupPins(dataPin, cmndPin, attPin, clockPin, 10);                                                            
  pinMode(A0, OUTPUT);                                    
  pinMode(6,  OUTPUT);
  pinMode(7,  OUTPUT);
  pinMode(8,  OUTPUT);
  pinMode(9,  OUTPUT);
  pinMode(10, OUTPUT);
  pinMode(11, OUTPUT);
  pinMode(12, OUTPUT);                                                      
  pinMode(A1, OUTPUT);                                   
}

void loop()
{
  data = Psx.read();                                     
  {
  digitalWrite(6,  data & Up );      
  digitalWrite(7,  data & Down );
  digitalWrite(8,  data & Left );
  digitalWrite(9,  data & Right );
  digitalWrite(10, data & PsxSqr );
  digitalWrite(11, data & PsxTri );
  digitalWrite(12, data & PsxX );                    
  digitalWrite(A0, data & PsxCrl);
  digitalWrite(A1, data & Start);
  }
// Serial.println(data);
}

Tried adding long and unsigned, now getting error:

psx_Lynx.ino: In function 'void loop()':
psx_Lynx:49: error: expected primary-expression before '.' token
psx_Lynx:50: error: expected unqualified-id before '{' token
psx_Lynx:50: error: expected `;' before '{' token

nm, I broke include. Put it back and now I can upload without error but I am still not getting the unsigned byte running the LEDs.

serial monitor shows:
Left = 1, Down = 2, Right = 4, Up = 8, Start = 16, Select (not used) = 128, square = 256 (which should be same as 0x0100), X = 512, circle = 1024, and triangle = 2048

Since the number are passed correctly, somewhere the data is handled as unsigned variable. There is no negative number from controller at all. Yet somehow the and isn't correctly comparing the predefined variable with data variable

Maybe psx.h or psx.cpp needs to be edited?

PS need to turn off smoke alarm, my brain is fried and the smoke from my ears tripped the alarm.

Bed time for me.

Nothing obviously wrong with the code, so I'm left wondering whether the inputs are what you expect. You haven't shown us any trace output at any point. There's no rate limiting on the trace output so it will spew out as fast as the serial port can run, which won't be convenient to post, but please try to extract a section showing the value of data as you press and release a single button which you expect to turn on an LED, and tell us which button you were using.

Oh, and you ought to get rid of the '{' after the call to Psx.read(), and the corresponding '}' at the bottom of the function. There is no reason to enclose those digitalWrite() statements in a code block, and it gives the impression that you have left an incomplete control clause in there.

Looks to me that Ardiuno isn't doing the "and" part right with value higher than a byte.

if that is the hypothesis you should write a sketch for this to verify it . something like below or a bit more complex one (looping etc)

void setup()
{
  serial.begin(115200); // we want to know it fast ;)
  int a = somevalue;
  int b = anothervalue;
  int e = expectedvalue;

  int c = a & b;
  if (c != e) Serial.println("failed");
  }
void loop(){}
{/code]

This:

long unsigned int data = 0;

should be this (like in psx.h):

unsigned int data = 0;

PeterH:
Nothing obviously wrong with the code, so I'm left wondering whether the inputs are what you expect. You haven't shown us any trace output at any point. There's no rate limiting on the trace output so it will spew out as fast as the serial port can run, which won't be convenient to post, but please try to extract a section showing the value of data as you press and release a single button which you expect to turn on an LED, and tell us which button you were using.

8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
8
2048
2048
2048
2048
2048
2048
2048
2048
2048
2048
2048
2048
2048
2048
2048
2048
2048
2048
2048
2048
1024
1024
1024
1024
1024
1024
1024
1024
1024
1024
1024
1024
1024
1024
1024
1024
1024
1024
1024
1024

I pressed up, then triangle, then circle on the PSX pad. Only up got LED to light up.

Oh, and you ought to get rid of the '{' after the call to Psx.read(), and the corresponding '}' at the bottom of the function. There is no reason to enclose those digitalWrite() statements in a code block, and it gives the impression that you have left an incomplete control clause in there.

Fixed, thanks. One less set of brackets to keep track since it's not needed.

matchy:
This:

long unsigned int data = 0;

should be this (like in psx.h):

unsigned int data = 0;

Changed. I'm still missing something somewhere.

I liked BASIC back then it was easier to figure out a missing setting between 1 byte and 1 word variable container.

Maybe I'll try splitting the data container into 2 separate high and low byte containers? The lower byte represents the directional plus select and start button being pressed in binary format: (select)(not used)(not used)(start)(up)(right)(down)(left) for example binary data 00010001 would indicate both left and start button is pressed at once. 2 digits are not used at all in digital controller. The high byte would be used for (L2)(R2)(L1)(R1)(triangle)(circle)(X)(square). if I can separate that in dataLow and dataHigh and do boolean operation it should work properly with turning on LEDs.

And it works.

#include <Psx.h>                                          // Includes the Psx Library 

#define dataPin 2
#define cmndPin 3
#define attPin 4
#define clockPin 5

#define Up 0x08
#define Down 0x02
#define Left 0x01
#define Right 0x04
#define PsxSqr 0x01
#define PsxTri 0x08
#define PsxX 0x02
#define PsxCrl 0x04
#define Start 0x10


Psx Psx;                                               
byte dataLow = 0;
byte dataHigh = 0;
unsigned int data = 0;                                 

void setup()
{

  Psx.setupPins(dataPin, cmndPin, attPin, clockPin, 10); 

  pinMode(A0, OUTPUT);                                   
  pinMode(6,  OUTPUT);
  pinMode(7,  OUTPUT);
  pinMode(8,  OUTPUT);
  pinMode(9,  OUTPUT);
  pinMode(10, OUTPUT);
  pinMode(11, OUTPUT);
  pinMode(12, OUTPUT);                                                      
  pinMode(A1, OUTPUT);                                  
}

void loop()
{
  int data = Psx.read();                                      
  dataHigh = data / 256;
  dataLow = data;

  digitalWrite(6,  dataLow & Up );      
  digitalWrite(7,  dataLow& Down );
  digitalWrite(8,  dataLow & Left );
  digitalWrite(9,  dataLow & Right );
  digitalWrite(10, dataHigh & PsxSqr );
  digitalWrite(11, dataHigh & psxTri );
  digitalWrite(12, dataHigh & PsxX );                    
  digitalWrite(A0, dataHigh & PsxCrl );
  digitalWrite(A1, dataLow & Start );
}

Defined dataLow and dataHigh as a byte rather than integar, divided the source data by 256 to get high byte (fraction is ignored), and just dumped the data into low byte (anything over 256 is lost) Not quite what I originally planned but it works.