Skip to content
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: sensor: adxl362: Bug fix for q31_t conv #80331

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vladislav-pejic
Copy link
Contributor

This is a bug fix for adxl362_temp_convert_q31 and adxl362_accel_convert_q31 functions.
Functions are used to convert samples received from sensor to q31_t format when RTIO stream is used.

Signed-off-by: Vladislav Pejic [email protected]

MaureenHelm
MaureenHelm previously approved these changes Oct 23, 2024
drivers/sensor/adi/adxl362/adxl362_decoder.c Show resolved Hide resolved
@@ -102,6 +111,7 @@ static int adxl362_decode_stream(const uint8_t *buffer, struct sensor_chan_spec
memset(data, 0, sizeof(struct sensor_three_axis_data));
data->header.base_timestamp_ns = enc_data->timestamp;
data->header.reading_count = 1;
data->shift = 18;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a pretty huge shift, whats the temp range here?

-256 to 256 C would be a shift of 8 which I'd think would be fully inclusive of the temp range of the part.

Copy link
Contributor Author

@vladislav-pejic vladislav-pejic Oct 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Output of temp sensor is in mC. Sensitivity is 65mC/LSB and it is 12 bit output so scale is 65*2048=133.120. Because of that we need 2^18. This is how I understood the data sheet and that corresponds with values received from the sensor.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah so at most the output will be ~ -133.120 to 133.120, which is fine and fits in the -256 to 256 scale a shift of 8 gets you with q31, leaving more bits for the fractional part of the fixed point here will get you better precision in the conversion I'd think.

so your Q scaler would then be 1.0/(1.0/0.065)*pow(2,31)/pow(2,8) which is 545259.52 or rounded to 545260

2048*545260 = 1116692480

Converting to C by dividing by the fractional part of the fixed point (31-8 = 23, pow(2,23)) would get us...

1116692480/pow(2,23) = 133.120 which is exactly right

Copy link
Contributor Author

@vladislav-pejic vladislav-pejic Oct 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did it like this at first but when I print the result in accel_polling application (I added temp print same as acceleration) I get 2.34.... value. With shift 18 I get 35 or 36 which is the correct temperature (this is die temp). Is the value 2.34... ok?

Copy link
Collaborator

@teburd teburd Oct 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you were doing something like...?

#include <zephyr/dsp/print_format.h>

q31_t temp = 1116692480;
printk("temp is %" PRIq", PRIq_arg(temp, 4, 8));

If so, and you aren't getting 133.120 then there's likely a bug in the print formatting helpers that we need to look at, maybe @yperess can help here as he's the author of those.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually use this website to verify the fixed-point calcs: https://chummersone.github.io/qformat.html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was my mistake. I think that is ok now. I am using shift of 8 and it is printing correct results.

@@ -121,6 +131,7 @@ static int adxl362_decode_stream(const uint8_t *buffer, struct sensor_chan_spec
memset(data, 0, sizeof(struct sensor_three_axis_data));
data->header.base_timestamp_ns = enc_data->timestamp;
data->header.reading_count = 1;
data->shift = range_to_shift[enc_data->selected_range];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

int64_t micro_ms2 = data_in * SENSOR_G / range_to_scale[range];

*out = CLAMP((micro_ms2 + (micro_ms2 % 1000000)), INT32_MIN, INT32_MAX);
*out = data_in * qscale_factor[range];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

This is a bug fix for adxl362_temp_convert_q31 and
adxl362_accel_convert_q31 functions.
Functions are used to convert samples received from
sensor to q31_t format when RTIO stream is used.

Signed-off-by: Vladislav Pejic <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Sensors Sensors platform: ADI Analog Devices, Inc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants