Skip to content

Update adafruit_vl53l1x.py #21

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

Closed
wants to merge 4 commits into from
Closed

Conversation

JohnFFlanagan
Copy link

I am fairly new to CircuitPhython so I may have missed something important here.

I attempted to use the roi_xy and roi_center functions on the latest CircuitPhython I loaded on my RP Pico W board.
It complained about setters and getters. I added the changes that I found from reading the documentation.

As it turns out, changing the values didn't do what I wanted. And without the SPAD table like Spark Fun has in their Arduino driver you don't know what to use for the SPAD anyway.

/Table of Optical Centers
*
* 128,136,144,152,160,168,176,184, 192,200,208,216,224,232,240,248
* 129,137,145,153,161,169,177,185, 193,201,209,217,225,233,241,249
* 130,138,146,154,162,170,178,186, 194,202,210,218,226,234,242,250
* 131,139,147,155,163,171,179,187, 195,203,211,219,227,235,243,251
* 132,140,148,156,164,172,180,188, 196,204,212,220,228,236,244,252
* 133,141,149,157,165,173,181,189, 197,205,213,221,229,237,245,253
* 134,142,150,158,166,174,182,190, 198,206,214,222,230,238,246,254
* 135,143,151,159,167,175,183,191, 199,207,215,223,231,239,247,255

* 127,119,111,103, 95, 87, 79, 71,  63, 55, 47, 39, 31, 23, 15, 7
* 126,118,110,102, 94, 86, 78, 70,  62, 54, 46, 38, 30, 22, 14, 6
* 125,117,109,101, 93, 85, 77, 69,  61, 53, 45, 37, 29, 21, 13, 5
* 124,116,108,100, 92, 84, 76, 68,  60, 52, 44, 36, 28, 20, 12, 4
* 123,115,107, 99, 91, 83, 75, 67,  59, 51, 43, 35, 27, 19, 11, 3
* 122,114,106, 98, 90, 82, 74, 66,  58, 50, 42, 34, 26, 18, 10, 2
* 121,113,105, 97, 89, 81, 73, 65,  57, 49, 41, 33, 25, 17, 9, 1
* 120,112,104, 96, 88, 80, 72, 64,  56, 48, 40, 32, 24, 16, 8, 0 Pin 1
*
* To set the center, set the pad that is to the right and above the exact center of the region you'd like to measure as your opticalCenter*/

Proposed changes
@JohnFFlanagan
Copy link
Author

I am sorry but I can't understand the failed checks. What is the "black" fail?

@caternuson
Copy link
Collaborator

caternuson commented Jan 2, 2025

It's a code format check:
https://learn.adafruit.com/creating-and-sharing-a-circuitpython-library/black

Also - it may help to first open a new issue for whatever is happening and describe what is not working, provide simple example code to demonstrate issue, etc.

Change single to double quotes
Fixed last update that only fixed first issue.
After LOTS of phython issue got "black" to run.
@JohnFFlanagan
Copy link
Author

I want to thank [caternuson] very much for his help. Without his help I would never have known what I did wrong.

@FoamyGuy FoamyGuy mentioned this pull request May 21, 2025
@FoamyGuy
Copy link
Contributor

@JohnFFlanagan thanks for this fix! Sorry that it did not get reviewed sooner.

I ended up merging #25 which contains essentially the same fix though with different values for the byte order which doesn't actually matter since only a single byte is being encoded and decoded anyway. Since "big" byte order seems to be the default it made sense to me to go with that instead of "little" even though it makes no functional difference and could perhaps even be omitted entirely falling back on the default value.

It took me a bit to understand how these values worked and the table that you posted here in the PR comment was immensely helpful to my finally getting it. I ended up putting that table into the docstring for roi_center so that it is visible in the documentation and code for this driver.

Thanks again for submitting this and the surrounding information as well as for working through the actions checks.

@FoamyGuy FoamyGuy closed this May 21, 2025
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.

3 participants