RobTillaart / AS5600

Arduino library for AS5600 magnetic rotation meter
MIT License
109 stars 20 forks source link

Set position when using Cumulative Position #30

Closed daniel-frenkel closed 1 year ago

daniel-frenkel commented 1 year ago

I really like the way you added the cumulative position feature, it's really neat and thank you for taking the time to do all of this.

Rather than resetting the position to 0, I'd like to set my own position but am having some trouble implementing the feature and cannot figure out why it won't work as expected.

In the Source file, I added this

int32_t AS5600::setPosition(int32_t pos)
{
  _lastPosition = 0;
  _position  = pos;
  return _position;
}

And in my Arduino sketch, I run this line of code to set the position to begin at "777"

as5600.setPosition(777);

However, when running the AS5600_position example, it will not set to "777" when starting the loop. Here is the full sketch:

#include "AS5600.h"
#include "Wire.h"

AS5600 as5600;   //  use default Wire

void setup()
{
  Serial.begin(115200);
  as5600.begin(10, 9); //ESP32
  as5600.setDirection(AS5600_CLOCK_WISE);  // default, just be explicit.
  delay(1000);
  as5600.setPosition(777); //  NEW CODE to set the initial value to 777
}

void loop()
{
  static uint32_t lastTime = 0;

  //  set initial position
  as5600.getCumulativePosition();

  //  update every 100 ms
  if (millis() - lastTime >= 100)
  {
    lastTime = millis();
    Serial.println(as5600.getCumulativePosition());
  }
}

Any idea what I may be doing wrong? Thanks again

RobTillaart commented 1 year ago

Thanks, I will try to dive into this asap however some other tasks have Priority (with capital p).

RobTillaart commented 1 year ago

(added syntax highlighting in your post)

Analysis

You set lastPosition to 0 so you create a difference with the absolute internal position of the AS5600 (value)

Then get getCumulativePosition() executes the line _position = _position - _lastPosition + value; (or one of the other three paths) So it will add the difference.

Example:

So if the AS5600 did not move you get a wrong value in position.

If the AS5600 did move it would also add the difference, easy to see if it moved from X => X+1 Then _position would become 777 + X + 1;

So I think your function should not change _lastPosition and should be

int32_t AS5600::setPosition(int32_t pos)
{
  _position  = pos;
  return _position;
}

Can you give it a try?

RobTillaart commented 1 year ago

Note to myself: Given that it looks a lot like resetPosition() I think this should be changed to

.cpp

int32_t AS5600::resetPosition(int32_t pos)
{
  int32_t old = _position;
  _position  = pos;
  return old;
}

.h

int32_t resetPosition(int32_t pos = 0);   //  default 0 for backwards compatibility.
RobTillaart commented 1 year ago

Had a quick test, and a minor fix on your code

#include "AS5600.h"
#include "Wire.h"

AS5600 as5600;   //  use default Wire

void setup()
{
  Serial.begin(115200);
  as5600.begin(10, 9); // ESP32    // (my test 14,15)
  //  as5600.setAddress(0x40);     // (my test)

  as5600.setDirection(AS5600_CLOCK_WISE);  // default, just be explicit.
  delay(1000);

  as5600.getCumulativePosition();  // <<<<
  as5600.resetPosition(777); //  NEW CODE to set the initial value to 777   //  <<<  patched your function in **resetPosition()**
}

void loop()
{
  static uint32_t lastTime = 0;

  //  set initial position
  as5600.getCumulativePosition();

  //  update every 100 ms
  if (millis() - lastTime >= 100)
  {
    lastTime = millis();
    Serial.println(as5600.getCumulativePosition());
  }
}

The extra call to as5600.getCumulativePosition(); sets the lastPosition to the position of the internal value of the sensor.

This is not as nice as one would like so I will check how to include the update of lastPosition in resetPosition()

RobTillaart commented 1 year ago

This works, I will create a new develop branch including this functionality. Please verify and test it.

.cpp


int32_t AS5600::resetPosition(int32_t pos)
{
  _lastPosition = readReg2(AS5600_RAW_ANGLE) & 0x0FFF;
  int32_t old = _position;
  _position = pos;
  return old;
}

.h

int32_t resetPosition(int32_t pos = 0);   //  default 0 for backwards compatibility.
RobTillaart commented 1 year ago

mmm, as this is a breaking change I must take some time to rethink the consequences.

(update, develop branch is created and build is running)

RobTillaart commented 1 year ago

added the function Int32_t resetCumulativePosition(int32_t position = 0); that resets the internal lastPosition by default.

The function resetPosition(int32_t position) does not reset the internal lastPosition.

The difference is subtile and I'll try to explain.

Give it a try

#include "AS5600.h"
#include "Wire.h"

AS5600 as5600;   //  use default Wire

void setup()
{
  Serial.begin(115200);
  as5600.begin(10, 9); //ESP32

  as5600.setDirection(AS5600_CLOCK_WISE);  // default, just be explicit.
  delay(1000);
  as5600.resetCumulativePosition(777);   //  <<<<<<<<<<<<<<<<<<
}

void loop()
{
  static uint32_t lastTime = 0;

  //  set initial position
  as5600.getCumulativePosition();

  //  update every 100 ms
  if (millis() - lastTime >= 100)
  {
    lastTime = millis();
    Serial.println(as5600.getCumulativePosition());
  }
}
daniel-frenkel commented 1 year ago

That appears to work great, I will continue testing it but so far it's working perfectly. Thank you for all work you just did and the work you put into this library. It's very very appreciated.

The reason I requested this feature is to track the step count of my stepper motor. This feature allows me to sync up the AS5600 with the steps of the stepper motor which then makes it extremely easy to detect missed steps on the motor. It is a very nice feature to have.

RobTillaart commented 1 year ago

Tracking steppers is a good application for this sensor.

I will leave the code as is for now, and look at it tomorrow with a fresh pair of eyes. No changes are expected and unless there is something breaking I will merge it.

Is the readme file clear enough to understand the difference between the two reset() functions? If you have remarks I can include them before the merge. (only if you have time)

Regards, Rob

RobTillaart commented 1 year ago

PS, please note I used your code as an example. Hope you are OK with that.

RobTillaart commented 1 year ago

Released the 0.3.6 and closed this issue. Feel free to reopen it if the issue is not solved, or reopen a new one.