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

A small lack of precision—a dynamic and/or configurable target height tolerance is necessary #7

Closed
aienabled opened this issue Nov 13, 2020 · 3 comments

Comments

@aienabled
Copy link

aienabled commented Nov 13, 2020

Hello!
For me, the default height tolerance results in overshooting by 8-18 mm. Sometimes it's performing a separate additional drive-up command after stop (e.g. raised to 1198 instead of 1080 mm; stopped around 1080 but then issued another raise command).
Perhaps the tolerance constant in the has_reached_target check should be configurable. I believe no single magic number will be good here as it depends on the drive speed and the drive speed depends on various factors such as the desk load (reportedly Idasen drive speed is two times slower when under a maximum specified load of around 70 kg for EU model) or the drive power.

In my case, changing the constant from 20 to 75 improved precision significantly—it's accurate up to 3 mm now (and consistenly within 1 mm in the most cases) instead of 8-18 mm for both lowering and raising the desk. By the way, the reported peak speed is 62.08 but I'm not sure if it's accurate (perhaps the script should measure the speed itself instead of trusting to the desk Bluetooth reporting?) as it takes 11 seconds to drive from 1080 to 670 mm in my case so the actual speed is around 37 mm/second (though it takes about two seconds to reach the max velocity so the peak speed is definitely higher). Anyway, here is my log after changing the height tolerance constant from 20 to 75:

python main.py --stand
...
Stopping at height: 1074.8mm (target: 1080.0mm)
Stopping at height: 1076.8mm (target: 1080.0mm)
Stopping at height: 1079.2mm (target: 1080.0mm)
...
python main.py
...
Initial height: 1080.9mm
...
python main.py --sit
...
Stopping at height: 675.8mm (target: 670.0mm)
Stopping at height: 673.3mm (target: 670.0mm)
Stopping at height: 671.0mm (target: 670.0mm)
...
python main.py
...
Initial height: 669.7mm

I think for the best precision the height tolerance should depend on the actual desk speed. Plus additional configurable constant.

Additionally, I think logging for every height event would be useful. I've added a line to do this:

print("Current height: {}mm (target: {}mm) speed {}mm/s".format(rawToMM(height), rawToMM(target), rawToSpeed(speed)))

it should be right after these lines:

def _move_to(sender, data):
       global count
       height, speed = struct.unpack("<Hh", data)
       count = count + 1

Also, it would be useful to log the final height after the height adjustment procedure completed so it's no longer necessary to call python main.py to get it.

Regards!

@aienabled aienabled changed the title A small lack of precision A small lack of precision—a dynamic and/or configurable target height tolerance is necessary Nov 13, 2020
@rhyst
Copy link
Owner

rhyst commented Nov 13, 2020

Thanks for the feedback!

Working out how often to send move commands from the speed sounds is a cool idea to experiment with. For now I have done as you suggested and added a height_tolerance config option.

I also added the additional height logging, does that achieve the goal of logging the actual final height?

@aienabled
Copy link
Author

Thank you! The height_tolerance option is missing from the config.yaml file but when I've added it here it worked fine. One small issue: you've mentioned in the README file that it's measured in mm but actually it's measured in tenths of a millimeter (the raw form for Idasen/Linak controllers). It would be better to change the height_tolerance to mm for consistency's sake (so the default config value would be 2.0).

The logging during operation is good now however the final height logging is missing. Currently I have to call python main.py to get the actual final height. E.g.:

python main.py --stand
Connecting
Connected c0:e0:28:0c:80:91
Initial height: 1001.4mm
Current height: 1001.6mm (target: 1080.0mm) speed 5.92mm/s
...
...
Current height: 1073.5mm (target: 1080.0mm) speed 62.08mm/s
Stopping at height: 1073.5mm (target: 1080.0mm)
Disconnecting
Current height: 1075.4mm (target: 1080.0mm) speed 62.08mm/s
Stopping at height: 1075.4mm (target: 1080.0mm)
Disconnecting
Lost connection with c0:e0:28:0c:80:91
Disconnecting
Disconnected


python .\main.py
Connecting
Connected c0:e0:28:0c:80:91
Initial height: 1079.5mm
Lost connection with c0:e0:28:0c:80:91
Disconnecting
Disconnected
...

As you can see the last reported height was 1075.4 but the final height is 1079.5 mm.
Reporting the final height may help when "calibrating" the tolerance number. Though I realize that it may cause additional latency for the script shutdown that may be not desired.

Regards!

@rhyst
Copy link
Owner

rhyst commented Nov 21, 2020

In the bleak branch I have now added a short pause and then printing the final height, which looks to be correct to me. I also corrected the units of the height_tolerance to mm, thank you for pointing that out.

I'm going to close this issue now but feel free to reopen if necessary.

@rhyst rhyst closed this as completed Nov 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants