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

WIP: refactor sensor code #344

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

ofosos
Copy link

@ofosos ofosos commented Aug 17, 2023

Heya fellas!
This is a bit bigger.

The intention of this PR is to seperate the sensor code into two parts.

One part is very low level and only deals with microseconds, pins, and interrupts. It also knows how to sequence, interleave, or parallelize the sensors. This is now tofmanager.h/.cpp.

The second part is now DistanceSensor. This part of the code know how to convert microseconds into the final result values, including computing the distance from the time of flight and including applying any offsets (handlebar).

The two parts communicate via a FreeRTOS Queue. The low-level part runs in a dedicated FreeRTOS task on core0. The idea behind this is, to create some freedom to tinker with new and interesting ways to work the existing sensors. One idea is to introduce some calibration routines, another idea is to try to accelerate the data gatherin by making it dynamic. One possible use case could be dooring zone detection. Despite all grand new ideas, I think the new code is an improvement and easier to understand.

This is not finished and it probably needs two more weeks of work. Right now the UI is "airworthy" and can be used by normal people. I ran the sensor for 2h today and had no OOM conditions. I also didn't see any crashes during these two hours.

The following things are missing right now: The raw data / CSV is not verified and I think it could be badly broken. Some things (like missing interrupt detection) are missing, I will review these things in the coming weeks. Bluetooth has been disabled (I didn't want to share core0). The low level code uses 64bit atomic unsigned integers for time measurement, and I think the high level part does not properly reflect that.

I think there are also more things missing. I will review the CSV output this week and fix any oversights. In the following two or three weeks I want to wrap up work on this one.

Please don't be angry at me, because I took a lot of liberty in renaming things. I especially felt, that the name of a variable or parameter should reflect the physical unit and the purpose (e.g. distance or distance with offsets applied) should be obvious from a comment.

Additionally I upgraded the project to be C++20 compliant.

Fellas, I think this is a marvelous project and a really fun thing to hack on. Have a good evening.

Cheers, Mark

@ofosos ofosos marked this pull request as draft August 18, 2023 10:17
@amandel
Copy link
Member

amandel commented Sep 6, 2023

Hi Mark,
thanks for you PR. Changes are appreciated and appreciated that the existing code needs some more C++ etc. know how ;). You combined a lot of changes in one PR which makes it a bit hard to follow. As you have noticed, the current code is a mix of C/C++ of different versions/Arduino/ESP-IDF which is not intended but how it evolved.
The sensor reading was finally quiet stable and several errors we digged down had the cause in hardware failure and unexpected behavior of the hardware.
Happy to see how this goes on!
Kind regards, Andreas.

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

Successfully merging this pull request may close these issues.

2 participants