Go Down

Topic: library for AS5048A 14 bit magnetic position sensor (Read 1 time) 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
 


Please enter a valid email to subscribe

Confirm your email address

We need to confirm your email address.
To complete the subscription, please click the link in the email we just sent you.

Thank you for subscribing!

Arduino
via Egeo 16
Torino, 10131
Italy