adafruit / Adafruit_AS7341

Other
29 stars 18 forks source link

Add getters and conversion method to basic counts #9

Closed Lithimlin closed 3 years ago

Lithimlin commented 3 years ago

I've added getter methods for the values ATIME, ASTEP, TINT, and the gain. Said methods are then used in the new method toBasicCounts which converts the values returned by the ADC to "Basic Count" values as described in the Application Note (Section 3.1) for the AS7341.

None of the already existing code was changed.

Lithimlin commented 3 years ago

I'm not sure what went wrong here. The error message is not really telling of what actually happened...

siddacious commented 3 years ago

Hi there, thank you for your contribution. To get details about the failure you'll have to click the "Details" link next to the failing review:

image

then expand the node for the failing step, in this case the clang formatting:

image

In the expanded node you can see the diff that was created when clang-format was applied to the code from the PR. This check tests that the code to be formatted using clang-format and will fail if it sees that running clang-format would create changes:

image

There are more changes beyond what is in the screenshot. You can try to appease clang by manually applying the changes however I would recommend that you make clang-format do it by using the -i flag which applies the changes in place: clang-format -i Adafruit_AS7341.*

Please @ me if you have any questions

Lithimlin commented 3 years ago

Thanks. I had tried looking at the message but I missed the difference in spacing so I was confused what the problem was. Thanks a lot for the help!

Lithimlin commented 3 years ago

@siddacious I'm pretty sure that these documentation fails are not caused by my implementation...

siddacious commented 3 years ago

@Lithimlin you are correct! The doxygen errors were due to me merging code without docs :\

I've added the docs in question so it should stop complaining if you merge master into your branch/fork and re-push

edit: nope! please stand by

siddacious commented 3 years ago

ok @Lithimlin, master is back to normal. Please merge and push to your branch and it should pass unless there's any issues with your PR

Lithimlin commented 3 years ago

@siddacious This is ready to be merged

Lithimlin commented 3 years ago

I've added an example for the conversion to basic counts. Internally, it uses the newly implemented getter methods.

Lithimlin commented 3 years ago

@siddacious let me know if there's anything still missing. Otherwise it should be ready now.

siddacious commented 3 years ago

works great @Lithimlin , thanks for the contrib and apologies for the delay