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

Adds calibration offsets for tilt on DJT11LM sensor #6622

Merged
merged 4 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions src/converters/fromZigbee.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4911,10 +4911,20 @@ const converters1 = {
// data[1][bit16..bit31]: y
// data[0][bit0..bit15] : z
// left shift first to preserve sign extension for 'x'
const x = ((data['1'] << 16) >> 16);
const y = (data['1'] >> 16);
let x = ((data['1'] << 16) >> 16);
let y = (data['1'] >> 16);
// left shift first to preserve sign extension for 'z'
const z = ((data['0'] << 16) >> 16);
let z = ((data['0'] << 16) >> 16);

// raw accelrometer values
result.raw_x=x;
result.raw_y=y;
result.raw_z=z;
Copy link
Owner

Choose a reason for hiding this comment

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

Why would you still want to have the raw values if you have the calibrated ones?

Copy link
Contributor Author

@ghoz ghoz Dec 5, 2023

Choose a reason for hiding this comment

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

it is the only way I found to get the raw values for doing the actual calibration

Copy link
Owner

Choose a reason for hiding this comment

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

But you already have the raw values before doing any calibration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do have the raw values, but right now, they are not available to users, we use them as is to compute and expose angles.
In order to know what offset to set, we need a way for the user to get the values when the sensor is on a level surface to compensate the values
(eg sensor flat I get "raw_x":60,"raw_y":-13,"raw_z":-704, => I set in the options the offset for x to -60, y to 13
with the sensor on one side : I get "raw_x":-942,"raw_y":-11,"raw_z":225, => offset for z ~ -225 )
so to reiterate as a user in order to find the offsets I need the raw values one way or another

To do an automated calibration witout the raw values, we would have to ask the user to put the device on 2 perpendicular faces, and wait for the values to update between each change, then compute the offsets for each axis and push them in the config or somewhere else, but not sure how to approach this tbh

Copy link
Contributor Author

@ghoz ghoz Dec 5, 2023

Choose a reason for hiding this comment

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

in the latest iteration, renamed the raw_x to x_axis to follow the other accelerometer sensors .
and expose the calibrated values


// simple offset calibration
x=calibrateAndPrecisionRoundOptions(x, options, 'raw_x');
y=calibrateAndPrecisionRoundOptions(y, options, 'raw_y');
z=calibrateAndPrecisionRoundOptions(z, options, 'raw_z');
Copy link
Owner

Choose a reason for hiding this comment

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

I think it's better to call the options x (no raw_)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looking at other accelerometer based sensors I see we have a set of x,y,z_axis that are exposed
using https://github.com/Koenkk/zigbee-herdsman-converters/blob/v16.0.2/src/lib/exposes.ts#L760
such as https://github.com/Koenkk/zigbee-herdsman-converters/blob/v16.0.2/src/devices/third_reality.ts#L228
Do you prefer me do rename and expose them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh sorry you were talking about the option name
yes I can rename (the question for the raw values is still valid though)

Copy link
Owner

Choose a reason for hiding this comment

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

Please rename

Copy link
Contributor Author

@ghoz ghoz Dec 5, 2023

Choose a reason for hiding this comment

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

renamed the option
I think It might be confusing as they are offsets for the accelerometer underlying /raw values, not the exposed angle_x y z that are computed...


// calculate angle
result.angle_x = Math.round(Math.atan(x/Math.sqrt(y*y+z*z)) * 180 / Math.PI);
Expand Down
5 changes: 5 additions & 0 deletions src/devices/xiaomi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2424,6 +2424,11 @@ const definitions: Definition[] = [
vendor: 'Xiaomi',
description: 'Aqara vibration sensor',
meta: {battery: {voltageToPercentage: '3V_2850_3000'}},
options: [
exposes.options.calibration('raw_x'),
exposes.options.calibration('raw_y'),
exposes.options.calibration('raw_z'),
],
fromZigbee: [xiaomi.fromZigbee.xiaomi_basic, fz.DJT11LM_vibration],
toZigbee: [tz.DJT11LM_vibration_sensitivity],
exposes: [
Expand Down