Preliminaries

I have optical sensor ADNS2610 (see datasheet here). There are a few problems which it can be used to solve, but for now let us focus on getting an image from that sensor. This sensor is kind of a small camera which has 400 cpi (counts per inch) resolution. And the resulting image consists of 324 = 18 x 18 pixels. The following is from the linked datasheet (p.23):

So in order to read a frame I should follow the algorithm:

1. Set the LED to forced awake mode


2. Write anything to the Pixel Data register


3. Read 324 times 6-bit unsigned integer from that register

So below is my program.

Program

Object oriented approach and WriteRegister and ReadRegisters functions (slightly modified) are borrowed from this project by zapmaker.

Optical sensor is represented by a class:

ADNS2610 class

#ifndef ADNS2610_H
#define ADNS2610_H

#include <inttypes.h>

#include <Arduino.h>

//Addresses of the registers
#define CFG_REG_ADDR 0x00
#define STS_REG_ADDR 0x01
#define DY_REG_ADDR 0x02
#define DX_REG_ADDR 0x03
#define SQL_REG_ADDR 0x04
#define MXP_REG_ADDR 0x05
#define MNP_REG_ADDR 0x06
#define PXLSUM_REG_ADDR 0x07
#define PXLDAT_REG_ADDR 0x08
#define STRUPR_REG_ADDR 0x09
#define STRLWR_REG_ADDR 0x0A
#define INVPRD_REG_ADDR 0x0B

//Bits of the configuration register
#define CFG_REG_LED 0x00
#define CFG_REG_PWRDWN 0x06 
#define CFG_REG_RST 0x07

//Bits of the Pixel data register
#define PXLDAT_REG_SOF 0x07
#define PXLDAT_REG_VLD 0x06

#define N_PIXELS 324

class ADNS2610

 private :
 uint8_t _sclkPin;
 uint8_t _sdioPin; 

 protected :
 uint8_t ReadRegister( uint8_t address );
 void WriteRegister( uint8_t address, uint8_t data );

 public :
 ADNS2610( uint8_t sclkPin, uint8_t sdioPin ) :
 _sclkPin( sclkPin ),
 _sdioPin( sdioPin )
 
 pinMode( _sclkPin, OUTPUT );
 pinMode( _sdioPin, INPUT );
 
 ~ADNS2610( ) ;

 bool IsAwake( );
 signed char GetDX( );
 signed char GetDY( );
 uint8_t GetSQUAL( );
 uint8_t GetMaxPixel( );
 uint8_t GetMinPixel( );
 uint8_t GetPixelAverage( );
 uint8_t* GetImage( );

 void SetAwakeLED( );
 void SetNormalLED( );
;

#endif

WriteRegister function

void ADNS2610::WriteRegister( uint8_t address, uint8_t data )

 address 

ReadRegister function

uint8_t ADNS2610::ReadRegister( uint8_t address )

 address &= ~( 1 << 7 );//Specify data direction

 pinMode( _sdioPin, OUTPUT );

 /*Write the 1st byte: data direction (MSB)
 *and register's address*/
 for( int i = 7; i >= 0; i-- )
 
 digitalWrite( _sclkPin, LOW );
 digitalWrite( _sdioPin, address & ( 1 << i ) );
 digitalWrite( _sclkPin, HIGH );
 

 pinMode( _sdioPin, INPUT );

 /*Wait for 100 microseconds between address write operation
 *and read data operation (see p.15 of the datasheet)*/
 delayMicroseconds( 100 );

 uint8_t data = 0;

 /*Write the 2nd byte*/
 for( int i = 7; i >= 0; i-- )
 
 digitalWrite( _sclkPin, LOW );
 digitalWrite( _sclkPin, HIGH );

 data 

 /*Wait for 0.25 microseconds between read and either
 *read or write operations (see p.15 of the datasheet)*/
 delayMicroseconds( 1 );

 return data;

GetImage function

uint8_t* ADNS2610::GetImage()

 //Returns pixel map as an array
 SetAwakeLED();//Step 1. in the above algorithm

 WriteRegister( PXLDAT_REG_ADDR, 0x00 );//Step 2. in the above algorithm

 static uint8_t frame[ N_PIXELS ];
 for( int i = 0; i < N_PIXELS; i++ )//Step 3. in the above algorithm
 
 frame[ i ] = ReadRegister( PXLDAT_REG_ADDR ) & 0b00111111;
 

 SetNormalLED();

 return frame;

As you can see I return image as just an array of uint8_t values from a function.

Update

See also this post where write and read operations are discussed in detail.

As always any critic, advice, correction and idea would be appreciated.

The pixel reading loop

 for( int i = 0; i < N_PIXELS; i++ )

leaves me uncomfortable. The data sheet effectively says that some reads may return Data_Valid bit set to 0, and must be retried. Also, the only sure way to determine that the image is completed is to look at SOF bit.

The address setup functionality is shared between ReadRegister and WriteRegister. Consider factoring it out into its own (private) method.

While reading image into the static buffer is a technically valid decision, I recommend to read it into the caller provided buffer.

The // Write the second byte comment in ReadRegister is misleading. You are reading there.

As a side note, the clock stays low for a very short period of time. I don't know what the wire protocol is; if it is i2c, you'd want to monitor for clock to actually toggle high before reading. I don't know what the data sheet says about it, but I recommend to double check it.

Don't use #define for constants or "functions" (⧺ES.31).

⧺Enum.5 Don't use ALL_CAPS for enumerators and ⧺ES.9 Avoid ALL_CAPS names.

Writing identifiers with a leading underscore is a bad idea, and is generally dissuaded as a style. Note that it is legal for member names if (and only if) the next character is not a capital letter or another underscore.

 ~ADNS2610( ) ;

If you have a destructor that looks like this, don’t say it at all. The compiler knows it is “trivial” if you do not declare one. If you must mention it, you could declare it with =default but that still counts as a declaration and will suppress the generation of move semantics.

Many of the functions named “get…” seem like they are accessors. They should be const members. You have not used const anywhere in the code posted, so there are probably many places where it should be inserted.

address |= ( 1 << 7 );//Specify data direction

That might be clearer using binary literals, now that they are available. 0b10000000.

address & ( 1 << i )

Is the compiler smart enough to figure out that you shift the mask one position each time through the loop? It might generate better code as well as being clearer to state the intent more directly:

for (uint8_t mask= 0b1000'0000; mask; mask>>=1) { ⋯

(or 0x80 or a named constexpr — whatever is easier to read)

static uint8_t frame[ N_PIXELS ];
for( int i = 0; i < N_PIXELS; i++ )//Step 3. in the above algorithm

 frame[ i ] = something

prefer to use the range-for.

for (auto& x : frame) 
 x = something;