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

[-breakout-] AS7343 #989

Closed
tony1tf opened this issue Aug 19, 2024 · 27 comments · Fixed by #993
Closed

[-breakout-] AS7343 #989

tony1tf opened this issue Aug 19, 2024 · 27 comments · Fixed by #993

Comments

@tony1tf
Copy link

tony1tf commented Aug 19, 2024

I've been running Python code with your library (from breakout_as7343 import BreakoutAS7343) to show the colour channels as bar graphs. However, 3 channels show low readings, which are not analogue. With low light level on channels 515nm, 600nm and 690nm, the output from the sensor is 11.0. With a higher light level the three channels jump to 139.0. Other working channels show readings in the 1000's. Is this a problem with your driver code, or do I have a faulty sensor?

@alphanumeric007
Copy link

You can see what he's referring too here.
https://forums.pimoroni.com/t/as7343-tinkering/25150/21

@alphanumeric007
Copy link

alphanumeric007 commented Aug 25, 2024

I''ve been poking around in the Micro python Modules for this breakout.
Just to recap the three misreading channels are as follows:
515nm is F4
600nm is FXL
690nm is F7

Looking in the breakout_as7343.cpp file I found this.

    tuple[0] = mp_obj_new_float(reading.FZ);
    tuple[1] = mp_obj_new_float(reading.FY);
    tuple[2] = mp_obj_new_float(reading.FXL);
    tuple[3] = mp_obj_new_float(reading.NIR);

    tuple[4] = mp_obj_new_float(reading.F2);
    tuple[5] = mp_obj_new_float(reading.F3);
    tuple[6] = mp_obj_new_float(reading.F4);
    tuple[7] = mp_obj_new_float(reading.F6);

    tuple[8] = mp_obj_new_float(reading.F1);
    tuple[9] = mp_obj_new_float(reading.F5);
    tuple[10] = mp_obj_new_float(reading.F7);
    tuple[11] = mp_obj_new_float(reading.F8);

They end up being the third one in each of the groups of 4? Might be just a coincidence but it's all I have found so far. I have zero C skills. @Gadgetoid
https://github.com/pimoroni/pimoroni-pico/blob/main/micropython/modules/breakout_as7343/breakout_as7343.cpp

@tony1tf
Copy link
Author

tony1tf commented Aug 27, 2024

That's a great observation, but we don't seem to be getting any response from Pimoroni. The current problem makes it unfit for purpose in MicroPython.

@alphanumeric007
Copy link

I totally agree. I guess the next step is to ask for a refund / return?

@helgibbons @Gadgetoid

@ZodiusInfuser
Copy link
Member

I''ve been poking around in the Micro python Modules for this breakout. Just to recap the three misreading channels are as follows: 515nm is F4 600nm is FXL 690nm is F7

Looking in the breakout_as7343.cpp file I found this.
...
They end up being the third one in each of the groups of 4? Might be just a coincidence but it's all I have found so far.

I am unfamiliar with the chip but that section for sure is a coincidence.
This section in (https://github.com/pimoroni/pimoroni-pico/blob/main/drivers/as7343/as7343.hpp), however is less so:

    struct reading {
        // Cycle 1
        uint16_t FZ = 0;   // 428-480 nm
        uint16_t FY = 0;   // 534-593 nm
        uint16_t FXL = 0;  // 593-628 nm
        uint16_t NIR = 0;  // 849-903 nm

        uint16_t c1_vis_tl = 0;  // Visible top-left
        uint16_t c1_vis_br = 0;  // Visible bottom-right
        uint16_t c1_astatus = 0; // (c1_astatus >> 8) & 0b10001111
                                 // 0b10000000 = saturated
                                 // 0b00001111 = gain lowest nibble

        // Cycle 2
        uint16_t F2 = 0;   // 408-448 nm
        uint16_t F3 = 0;   // 448-500 mn
        uint16_t F4 = 0;   // 500-534 nm
        uint16_t F6 = 0;   // 618-665 nm

        uint16_t c2_vis_tl = 0;
        uint16_t c2_vis_br = 0;
        uint16_t c2_astatus = 0;

        // Cycle 3
        uint16_t F1 = 0;   // 396-408 nm
        uint16_t F5 = 0;   // 531-594 nm
        uint16_t F7 = 0;   // 685-715 nm
        uint16_t F8 = 0;   // 715-766 nm

        uint16_t c3_vis_tl = 0;
        uint16_t c3_vis_br = 0;
        uint16_t c3_astatus = 0;
    };

This struct is populated by these functions:

  void AS7343::read_fifo(uint16_t *buf) {
    uint16_t expected_results = read_cycles * 7;
    uint16_t result_slot = 0;

    while (i2c->reg_read_uint8(address, reg::FIFO_LVL) < expected_results) {
        sleep_ms(1);
    }

    while (i2c->reg_read_uint8(address, reg::FIFO_LVL) > 0 && expected_results > 0) {
        buf[result_slot] = i2c->reg_read_uint16(address, reg::FDATA);
        expected_results--;
        result_slot++;
    }
  }

  AS7343::reading AS7343::read() {
    reading result;
    read_fifo((uint16_t *)&result);
    return result;
  }

There is nothing in there that I can see that would specifically be incorrect for the 3rd data entry in each group/cycle, unless the struct itself is wrong. I am having a difficult time determining what FDATA actually returns though, like where do those vis_tl, vis_br and astatus values come from? And it seems we don't expose those values on the Micropython side.

@tony1tf
Copy link
Author

tony1tf commented Aug 28, 2024

Hi ZodiusInfuser
It's great that you are looking into this. Unfortunatey, I am totally reliant on the simpler microPython links, and my understanding of C++ and the very complex chip registers is entirely useless to help debug this problem. There's a lot more going on inside the sensor than is obvious at first. It would be great if someone who understands it could write an easy to understand data-sheet.
Tony

@alphanumeric007
Copy link

Phil Howard @Gadgetoid is I think, the one who originally coded it? I'm guessing he's on vacation. Things have been quiet "Actions" wise. ;) That's how it goes sometimes.
On the Pimoroni Forum, you get nudged to post said issue on GitHub, for a better response. In this case that doesn't seem to be happening?

@ZodiusInfuser
Copy link
Member

Phil Howard @Gadgetoid is I think, the one who originally coded it? I'm guessing he's on vacation.

Yes, Phil did code it, but following the manufacturer's documentation. He is actually off sick at the moment, not to mention dealing with a mountain of RP2350 related issues.

As far as I have been able to determine, our code for reading the data from the sensor is correct. It could well be that the sensor initialisation is wrong, but we have not been able to understand it. As we say on the product page:

this sensor is complex and you will likely need to study the datasheet and the documentation for an understanding of how it works. We've spun up some basic examples to show you how you can get raw numbers from the sensor, but additional work will be required to convert this into useful data like RGB values. Some uses may require calibration of the sensor and the addition of a diffuser layer, which is not included with our breakout.

Things have been quiet "Actions" wise. ;) That's how it goes sometimes.
On the Pimoroni Forum, you get nudged to post said issue on GitHub, for a better response. In this case that doesn't seem to be happening?

This is still the correct reporting process for software specific issues.

@Gadgetoid
Copy link
Member

Sorry, between summer holiday shenanigans and other responsibilities this had escaped my attention.

I ran your code from the forum post and seem to get nice, smooth motion on all colour channels using the LED torch on my phone and moving it closer/further away (inverse square law yay?).

I did momentarily get some sticking on F1, F2 and FZ which cleared when I reset/repowered the board.

I then turned on the on-board LEDs and everything went south... sticking on random channels, and what seems like a dynamic re-calibration causing the levels to suddenly be much, much different from what I was seeing before.

I swapped out the sensor and set the misbehaving one aside, and got a different set of weirdness with my other sensor.

I swapped back the one I set aside, and it was back to what I was seeing before.

Now, I'm back to my other sensor being weird.

I can't seem to find any predictable way to replicate this.

There's clearly something marginal awry with my code or my understanding of the sensor and I'm investigating.

@Gadgetoid
Copy link
Member

@tony1tf Accepting that you might have done this at some point organically, can you remove your sensor from Pico Explorer/Power, set it aside for a minute or so, and - without enabling the onboard LEDs - use another light source and see if it responds how you might expect?

I can't find anything fundamentally wrong with my code, nor anything that suggests LED power/current settings trigger any other feature. Nor can I seem to reliably replicate the weirdness I encountered, not for want of trying.

Another thing to try would be forcing a reset of the autozero feature, you can do this by adding the following lines before setting up the sensor but after setting up i2c:

i2c.writeto(0x39, bytes([0x80, 0b00000000]))
i2c.writeto(0x39, bytes([0xFA, 0b00000100]))

eg: in my case:

i2c = PimoroniI2C(sda=20, scl=21)
i2c.writeto(0x39, bytes([0x80, 0b00000000]))
i2c.writeto(0x39, bytes([0xFA, 0b00000100]))
as7343 = BreakoutAS7343(i2c)

@alphanumeric007
Copy link

I have the AS7343 onboard LED's turned off. My setup is on a Pico W Explorer (prototype). I added a reset button. Hitting reset doesn't change anything.
MicroPython v1.23.0, picow v1.23.0-1 on 2024-06-06; Raspberry Pi Pico W with RP2040

I can try a different setup and or uf2 if you want?
I have a Tufty I can repurpose for testing, or a breakout Garden setup.

@alphanumeric007
Copy link

Turning the onboard LED's on "current(4)" didn't change the three problem channels. All the others react differently, but not those three.

@Gadgetoid
Copy link
Member

This balmy setup unsticks everything but F1, F2 and F3 for me:

from breakout_as7343 import BreakoutAS7343
from pimoroni_i2c import PimoroniI2C
from picographics import PicoGraphics, DISPLAY_PICO_W_EXPLORER, PEN_P4
from pimoroni import Button
import time

i2c = PimoroniI2C(sda=20, scl=21)

as7343 = BreakoutAS7343(i2c)

# Soft reset
i2c.writeto(0x39, bytes([0xFA, 0b00001000]))
time.sleep(0.5)

# Bank 0
i2c.writeto(0x39, bytes([0xBF, 0]))

# Set AGAIN
i2c.writeto(0x39, bytes([0xC6, 9]))
as7343.set_gain(256)
as7343.set_measurement_time(33)  # Roughly 30fps at 16ms/measurement
as7343.set_integration_time(27800)
as7343.set_channels(18)

# Don't auto zero
i2c.writeto(0x39, bytes([0xDE, 255]))

# Set the FIFO map
i2c.writeto(0x39, bytes([0xFC, 0b01111111]))


# Enable things
i2c.writeto(0x39, bytes([0x80, 0b00011011]))


button_a = Button(12)
button_b = Button(13)
button_x = Button(14)
button_y = Button(15)

display = PicoGraphics(display=DISPLAY_PICO_W_EXPLORER, pen_type=PEN_P4, rotate=270)
display.set_font("bitmap8")
display.set_backlight(0.5)


WIDTH, HEIGHT = display.get_bounds()
#WIDTH = 240
#HEIGHT = 240
#the above call doesn't get this for the display_pico_explorer

print (WIDTH, HEIGHT)

BLACK = display.create_pen(0, 0, 0)

FZ = display.create_pen(0, 70, 255)
FY = display.create_pen(179, 255, 0)
FXL = display.create_pen(255, 190, 0)
NIR = display.create_pen(97, 0, 0)
F2 = display.create_pen(84, 0, 255)
F3 = display.create_pen(0, 192, 255)
F4 = display.create_pen(31, 255, 0)
F6 = display.create_pen(255, 33, 0)
F1 = display.create_pen(130, 0, 200)
F5 = display.create_pen(163, 255, 0)
F7 = display.create_pen(255, 0, 0)
F8 = display.create_pen(171, 0, 0)

WHITE = display.create_pen(255, 255, 255)


# Done above
#as7343.set_channels(18)
#as7343.set_gain(256)
#as7343.set_measurement_time(33)  # Roughly 30fps at 16ms/measurement
#as7343.set_integration_time(27800)


BAR_WIDTH = 16
BAR_SPACING = 4
MARGIN = display.measure_text("NIR") + 2
BAR_HEIGHT = WIDTH - MARGIN

OFFSET_LEFT = int((HEIGHT - 110 - ((BAR_WIDTH + BAR_SPACING) * 12)) / 2)

# Starting max values for auto-ranging
# From figure 8 of the datasheet
# F3 in pos 6 was 962 but gave overrange
# F5 in pos 10 was 1967 but also overrange
MAX_VALUES = [
    2711,
    4684,
    5970,
    13226,
    2371,
    5000,
    3926,
    4170,
    7760,
    5000,
    6774,
    1166
]

LABELS = [
    "FZ",
    "FY",
    "FXL",
    "NIR",
    "F2",
    "F3",
    "F4",
    "F6",
    "F1",
    "F5",
    "F7",
    "F8"
]

PLACE = [
    3,
    6,
    8,
    12,
    2,
    4,
    5,
    9,
    1,
    7,
    10,
    11
]


while True:
    display.set_pen(0)
    display.clear()
    readings = as7343.read()
    for i, reading in enumerate(readings):
        MAX_VALUES[i] = max(reading, MAX_VALUES[i])
        scaled = int(reading / MAX_VALUES[i] * BAR_HEIGHT)
#       y = i * (BAR_WIDTH + BAR_SPACING)
        y = PLACE[i] * (BAR_WIDTH + BAR_SPACING)
        y += OFFSET_LEFT

        display.set_pen(i + 1)
        display.rectangle(MARGIN, y, scaled, BAR_WIDTH)

        display.set_pen(WHITE)
        display.text(LABELS[i], 0, y + 1)

    display.update()
    if button_b.is_pressed:
        as7343.set_illumination_led(False)
    elif button_a.is_pressed:       
        as7343.set_illumination_current(4)
        as7343.set_illumination_led(True)
    elif button_x.is_pressed:
        as7343.set_illumination_current(10)
        as7343.set_illumination_led(True)
    time.sleep(0.01)

@Gadgetoid
Copy link
Member

Gadgetoid commented Sep 5, 2024

I've also ported the C code to MicroPython to make it easier to poke, you can find that here: https://gist.github.com/Gadgetoid/a7227b56f7d34277b3616225c5884976 (new lib, see below)

It's more or less a drop-in replacement. Save it as breakout_as7343_py.py and use from breakout_as7343_py import BreakoutAS7343

One crucial difference is that it starts measurements only when you ask for a reading (or if you call start_measurement).

@Gadgetoid
Copy link
Member

I see also that the datasheet has been updated to correct the order of some channels 🤦

@Gadgetoid
Copy link
Member

Okay to fix the channel order, fix the annoying ambiguity of the sensor readings, include visible light readings and make tweaking the library less of a headache I've done a full port to pure Python including the Pico Explorer example (which is very nice, please let me know how I should appropriately credit you for it):

https://github.com/pimoroni/as7343-micropython

Notably the readings are now a dict so they're much easier to pick into any order you want without headache-inducing magic numbers.

This code has, so far, been more reliable for me, with the small exception that F1, F2 and FZ are... misbehaving? These are the first FIFO entries in each of the three cycles which is... possibly relevant. Notably 139 is 0b10001011 and 11 is 0b00001011... so this looks like the status register and not the first reading that it's supposed to be 🤔

@Gadgetoid
Copy link
Member

I think I had made some assumptions about the order of data in the FIFO that just weren't correct, though I'm not quite sure how things ever quite worked. In truth I'm not sure they did, which explains the fantastic amount of confusion I encountered when trying to test the thing. I had eventually assumed that the spectral bands were simply too abstract for me to match them to object colours... but hindsight is 20/20 and I was clearly just missing my code being outright wrong.

I think which bands stick might have something to do with reads from the FIFO getting misaligned, or various features getting enabled/disabled. In short the ASTATUS entry - I think- is prepended to each cycle and the FD (Flicker Detect) is appended. This means you get these three times per 18 channel read, and they can cause all kinds of havoc with the data if not handled with care.

Sorry for the headache this caused you both. I'd appreciate some tinkering with the linked MicroPython library but I'm going to backport my fixes here anyway for C++ users (and our existing MicroPython module will benefit from those too).

@alphanumeric007
Copy link

Got It working once I found and copied the as7343.py file to my Pico. ;)
I'm liking the response. F1, F2 and FZ are missing though?

@Gadgetoid
Copy link
Member

Got the latest version of as7343.py? The fix for that should be pimoroni/as7343-micropython@ea5c565

@alphanumeric007
Copy link

@alphanumeric007
Copy link

alphanumeric007 commented Sep 6, 2024

Ops, a big sorry Phil. I rotated it 90' on my Pico Explorer W and it cut off the F 1, and F2. I had another look and saw that FZ was showing, and did some thinking. Then remembered un rotating it so it was right way round on my screen. My screen is 320 wide by 240 and the ribbon cable comes in from the right. On the Pico Explorer its 240x240 with the ribbon cable coming in from below. So sorry if this caused you grief. =(

My Pico Explorer W is an unreleased RP2040 W onboard version. Hel sent it to me to test it out and tinker with. My Tufty was already in use so I used this for tinkering with the AS7343.

EDIT: It looks like it was this line of code that messed up my screen position.
OFFSET_LEFT = int((HEIGHT - 110 - ((BAR_WIDTH + BAR_SPACING) * 12)) / 2)
Changed the 110 to 40 and all is good now. I am so liking the response. I have a 144 RGB LED strip Plasma Stick setup for my desktop lighting. I was just having fun switching the colors and intensity and watching the as7343 display follow suit. =)

@Gadgetoid
Copy link
Member

I was just having fun switching the colors and intensity and watching the as7343 display follow suit. =)

This kind of stuff makes progressing through the five stages of grief with a flaky library worthwhile 😆 I wish I could blame it entirely on the datasheet being wrong, but nope, mostly me confusing ASTATUS with FD (not that the datasheet appears very helpful on this front).

So sorry if this caused you grief. =(

I was way too asleep to worry about it 😆 we definitely need some more examples in the Python library repository, so hopefully we can cover some basic use cases, and screens are definitely much, much easier to understand.

I did much of my early dev work and testing on a Pi looking at numbers, so at some point my brain clearly just refused to comply. Trouble with working mostly alone is that if I become blind to a problem there's no helping me.

I've raised a PR to fix this bug on the Python library - pimoroni/as7343-python#12

And I'll update the Pico C stuff this morning!

@alphanumeric007
Copy link

A lot of the code I use is clip and pasted (from the examples) when and where I need it. I know what it does, but have no idea how it does it, lol.
I for one really apricate you sorting this out. This is one awesome light sensor.

@tony1tf
Copy link
Author

tony1tf commented Sep 6, 2024

Hi folks - back on the case today. I'll load the new library and see if it all looks good. One of my pre-retirement colleagues was measuring atmospheric pollution with a precision spectrometer and CCD camera, which I helped with. Since the AS7343 has quite narrow spectral bands, I wondered if it could be used in this way by 'peering' into the atmosphere, which is why I bought one. It will be great if it now gives accurate intensities from each filtered pixel. My programming knowledge is limited, which is why I have to rely on the abilities of Gadgetoid to get things going for me. Keep up the good work!

Gadgetoid added a commit that referenced this issue Sep 6, 2024
The astatus value was being interpreted as FZ, F2 and F1 causing bizarre
readings and all other channels to be shifted one place.

Additionally the datasheet has been updated to re-order F5, F7 and F8 to
F7, F8, F5, so these channels have been re-ordered accordingly.

Note: This will turn sensor output from confusing abject nonsense into
what looks like pretty reasonable colour data and fixes #989.
@tony1tf
Copy link
Author

tony1tf commented Sep 6, 2024

Everything seems OK now - thank you. I have added to the Pico Explorer example to interrogate the Y button to swap the label to nm, by adding another list called ORDER2. Do you want me to post it somewhere?
I note that in your readme file (which I now can't find) , the headings for max and mean are swapped.

@Gadgetoid
Copy link
Member

A PR against https://github.com/pimoroni/as7343-micropython would be helpful- updating the example accordingly!

I could have sworn I put some of the datasheet tables in a README somewhere, too, but I couldn't find it either! 😬

@tony1tf
Copy link
Author

tony1tf commented Sep 7, 2024 via email

Gadgetoid added a commit that referenced this issue Oct 7, 2024
The astatus value was being interpreted as FZ, F2 and F1 causing bizarre
readings and all other channels to be shifted one place.

Additionally the datasheet has been updated to re-order F5, F7 and F8 to
F7, F8, F5, so these channels have been re-ordered accordingly.

Note: This will turn sensor output from confusing abject nonsense into
what looks like pretty reasonable colour data and fixes #989.
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 a pull request may close this issue.

4 participants