library for AS5048A 14 bit magnetic position sensor

Hello,

I needed a lib for a magnetic position sensor and because i could not find any i did it myself...so look in the attachament

If any one can help to improve it then please reply.

AS5048.zip (6.32 KB)

Thanks for sharing this lib,

I had a quick look at it and have the following comments.

  1. version numbering
    #define AS5084A_LIB_VERSION "0.1.0"

  2. readability
    you need to add more spaces and white lines to make the code a bit more readable.

  3. some code tricky
    _data = (uint16_t)((_hB << 8) | _lB); // are you sure _hB << 8 become 16 bit before the | ??

sometimes it is simpler to write
_data = 256 * _hB + _1B;

(and please name them _highByte and _lowByte :slight_smile:

  1. comments
    Your code is overloaded with long comments that exceeds the code.
    Using good names for var #defines & functions prevents unneeded comments

(not reworked in all detail but just to get an idea)

//Read Registers included parity bit
#define SPI_READ_DIAGNOSTICS	0x7FFD 
#define SPI_REG_MAGNITUDE		0x7FFE  // Magnitude information after ATAN calculation
#define SPI_REG_ANGLE_INFO	 0xFFFF  // Angle information after ATAN calculation and zero position adder
...
uint16_t AS5048::getAngle(void)
{
  sendCMD(SPI_REG_ANGLE_INFO);  
  return (getData() & 0x3FFF); 
}

uint16_t AS5048::getMagnitude(void)
{
  sendCMD(SPI_REG_MAGNITUDE); 
  sendCMD(SPI_NOP);
  return getData();
}

uint16_t AS5048::getDiagnostics(void)
{
  sendCMD(SPI_READ_DIAGNOSTICS);
  sendCMD(SPI_NOP);
  return getData();
}

void AS5048::sendCMD(uint16_t cmd)
{
  //CheckSPI;//because i use the same library for SPI memory where SPCR = B01010000
  CS0(_devPin);
  mySPI.transferData(cmd >> 8);
  mySPI.transferData(cmd & 0xFF);
  CS1(_devPin);
}

uint16_t AS5048::getData(void)
{
  CS0(_devPin);
  _data = 256 * mySPI.transferData(0xFF); 
  _data += mySPI.transferData(0xFF);
  CS1(_devPin);

  if (_data & SPI_RESET_ERROR )    // define magic numbers  like 0x4000 also for the 65535 (0xFFFF)  that is also magic :)
  {
    sendCMD(SPI_REG_CLRERR);
    return 65535;
  }
  return _data;
}
  1. the code seems to be hard coded to PORTD while the sensor can (I assume) be connected to any pin.

  2. uint16_t getData(void); can be private I guess?

my 2 cents

Thank for reply,

I will try to implement your suggestions.

regarding point 4, this lib was made so that it can serve my application as best as possible

point 5 - it can be, but sometimes because the way the sensor works (you read the result of previous command) i think i will need the function public, if not i will make it private.

egarding point 4, this lib was made so that it can serve my application as best as possible

OK, getting a working version is always the first step. Gold-plating can be done afterwards :wink: