Go Down

Topic: library for AS5048A 14 bit magnetic position sensor (Read 967 times) previous topic - next topic

gvi70000

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.

robtillaart

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.

5) 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 :)


3) 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)
Code: (example) [Select]

//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;
}


4) the code seems to be hard coded to PORTD while the sensor can (I assume) be connected to any pin.

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

my 2 cents

Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

gvi70000

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.

robtillaart

Quote
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 ;)
Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Go Up