-
Notifications
You must be signed in to change notification settings - Fork 130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[driver] Adding AS5047 driver and example #1138
Conversation
hshose
commented
Mar 6, 2024
- driver
- example
- tested on hardware
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, just some nitpicks.
par ^= par >> 2; | ||
par ^= par >> 1; | ||
return ((par & 1) << 15) | (0x7fff & num); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the modm namespace which is quite generic. Would be good to move to struct as5047
or put into the relevant :math
module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure, how to move it into the struct?
If I declare `setMsbToEvenParity` as `static constexpr` in the `as5047` struct, I get an error saying `error: 'static constexpr uint16_t modm::as5047::setMsbToEvenParity(uint16_t)' used before its definition`
struct as5047
{
enum class Errorfl : uint16_t
{
Parerr = Bit2,
Invcomm = Bit1,
Frerr = Bit0,
};
MODM_FLAGS16(Errorfl)
enum class Prog : uint16_t
{
Progver = Bit6,
Progotp = Bit3,
Otpref = Bit2,
Progen = Bit0,
};
MODM_FLAGS16(Prog)
enum class Diaagc : uint16_t
{
Magl = Bit11,
Magh = Bit10,
Cof = Bit9,
Lf = Bit8,
};
MODM_FLAGS16(Diaagc)
static constexpr uint16_t
setMsbToEvenParity(const uint16_t num)
{
uint16_t par = 0x7fff & num;
par ^= par >> 8;
par ^= par >> 4;
par ^= par >> 2;
par ^= par >> 1;
return ((par & 1) << 15) | (0x7fff & num);
}
enum class Register : uint16_t
{
ReadNop = setMsbToEvenParity(((1 << 14) | 0x0000)),
ReadErrfl = setMsbToEvenParity(((1 << 14) | 0x0001)),
ReadProg = setMsbToEvenParity(((1 << 14) | 0x0003)),
ReadDiaagc = setMsbToEvenParity(((1 << 14) | 0x3FFC)),
ReadMag = setMsbToEvenParity(((1 << 14) | 0x3FFD)),
ReadAngleunc = setMsbToEvenParity(((1 << 14) | 0x3FFE)),
ReadAnglecom = setMsbToEvenParity(((1 << 14) | 0x3FFF)),
};
struct modm_packed Data
{
/// @return
constexpr float
getAngleRad() const
{
const uint16_t angle = data & 0x3fff;
return static_cast<float>(angle) / 16383.f * 2.f * std::numbers::pi_v<float>;
}
/// @return
constexpr float
getAngleDeg() const
{
const uint16_t angle = data & 0x3fff;
return static_cast<float>(angle) / 16383.f * 360.f;
}
/// @return
constexpr uint16_t
getAngleRaw() const
{
const uint16_t angle = data & 0x3fff;
return angle;
}
uint16_t data;
};
}; // struct as5047
I think this might be related to this:
The Standard should make clear that a constexpr member function cannot be used in a constant expression until its class is complete.
Do you have any suggestions?
Should I put everything into an as5047
namespace?
I don't think moving the function as is to the math:
is appropriate, since the function specifically calculates and sets bit 15 of a uint16_t
, if moved to math:
, a more generic version might be more appropriate. Also, other modm modules like etlcpp
have their own functions to calculate parity bits...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh, C++ amiright!1!!
You can put it into a modm::details
namespace then and use /// @cond
and /// @endcond
to block doxypress from indexing it (Examples). Prolly prefix the function with as5047_
to avoid collisions if you wanna be super careful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh, C++ amiright!1!!
🐈 No we are super careful 🐱
ad12c38
to
dd24d53
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!