fix: consider incand->card_num when checking opp lights

[unRARed]

Fixes #1867. Modifies how OPPIncand lights are numbered, so that the validation doesn't incorrectly determine duplicate light numbers for these boards.

Without this patch, OPP Cypress builds having incandescents used across multiple boards will run into this issue (unless they don't use the same numbered pin on any of the boards in the chain). The following videos demonstrate a game booting to attract mode with the patch in place:

opp-incand.mov

opp-lights-dup.mov

[cobra18t]

I think this is the wrong place to add a check like this. This is a generic platform-agnostic file and you are putting OPP specific info in it. We should find the problem on the OPP side and address it there.

[unRARed]

Yes, agreed. Seems like OPPIncand->number needs to produce the number differently, with the incand_card->card_num, but I don't know the codebase. Hopefully this at least helps to shine light on the problem.

[cobra18t]

Knowing this, I wonder if multiple boards worth of incand cards has ever been used on STM32 OPP. It looks like it would suffer the same fate. I agree that lumping the more complete number into OPPIncand->number would likely solve this problem, but I need to check if that would break anything else.

[unRARed]

I updated the PR to do just that, and the debugging seems identical:

But yes, now this is a more global change, so I also wonder about how it might affect other OPP code. Also, it seems like it might be worth including the chain number, too, given that seems like the new best practice?

[cobra18t]

Yes, you need chain_serial in there or STM32 may never work for this type of use case.

Take a look at opp_coil.py as a reference. There it uses the full x-y-z for the number. However, anywhere it needs to access a portion of that, it uses the "split()" method to extract it. The same needs to happen if you change Incand to the full number.

[unRARed]

hmmm, well I'm not really a Python dev and since I don't have or plan to have the STM32 setup, trying to implement this properly (for multiple OPP hardware configs) seems like a stretch for me. I also wonder if this doesn't actually work fine physically, and it's perhaps only the validation check that's not doing enough for the older OPP setup we have. We'd need to wire up some lamps to 0-0 and 2-0 (with the example config) and see if they can be controlled individually or if the system treats them both as a common 0. If it's just the duplicate light check that's a real problem, the naive fix might be the path of least resistance. Or maybe a cli flag to disable specific validation(s) even?

[unRARed]

btw, messaged you on pinside

[unRARed]

With this latest change, the output of the "key" variable from ./mpf/devices/light.py looks like this against our real config (6 OPP Cypress boards) - truncated for brevity:

21:20:02.505 : INFO [BallController] Initial balls found: 5
**********driver.number********
('opp',
 '/dev/cu.usbmodem101-0-0',
 <class 'mpf.platforms.opp.opp_incand.OPPIncand'>)
**********driver.number********
**********driver.number********
('opp',
 '/dev/cu.usbmodem101-0-1',
 <class 'mpf.platforms.opp.opp_incand.OPPIncand'>)
**********driver.number********
**********driver.number********
('opp',
 '/dev/cu.usbmodem101-0-2',
 <class 'mpf.platforms.opp.opp_incand.OPPIncand'>)
**********driver.number********

... etc. ...

**********driver.number********
('opp',
 '/dev/cu.usbmodem101-2-0',
 <class 'mpf.platforms.opp.opp_incand.OPPIncand'>)
**********driver.number********
**********driver.number********
('opp',
 '/dev/cu.usbmodem101-2-1',
 <class 'mpf.platforms.opp.opp_incand.OPPIncand'>)
**********driver.number********
**********driver.number********
('opp',
 '/dev/cu.usbmodem101-2-2',
 <class 'mpf.platforms.opp.opp_incand.OPPIncand'>)
**********driver.number********

... etc. ...

**********driver.number********
('opp',
 '/dev/cu.usbmodem101-3-8',
 <class 'mpf.platforms.opp.opp_incand.OPPIncand'>)
**********driver.number********
**********driver.number********
('opp',
 '/dev/cu.usbmodem101-3-9',
 <class 'mpf.platforms.opp.opp_incand.OPPIncand'>)
**********driver.number********
**********driver.number********
('opp',
 '/dev/cu.usbmodem101-3-10',
 <class 'mpf.platforms.opp.opp_incand.OPPIncand'>)
**********driver.number********

... etc. ...

**********driver.number********
('opp',
 '/dev/cu.usbmodem101-4-8',
 <class 'mpf.platforms.opp.opp_incand.OPPIncand'>)
**********driver.number********
**********driver.number********
('opp',
 '/dev/cu.usbmodem101-4-9',
 <class 'mpf.platforms.opp.opp_incand.OPPIncand'>)
**********driver.number********
**********driver.number********
('opp',
 '/dev/cu.usbmodem101-4-10',
 <class 'mpf.platforms.opp.opp_incand.OPPIncand'>)
**********driver.number********

... etc. ...

**********driver.number********
('opp',
 '/dev/cu.usbmodem101-5-8',
 <class 'mpf.platforms.opp.opp_incand.OPPIncand'>)
**********driver.number********
**********driver.number********
('opp',
 '/dev/cu.usbmodem101-5-9',
 <class 'mpf.platforms.opp.opp_incand.OPPIncand'>)
**********driver.number********

... etc. ...

It's loading to attract. The question is just with the light->number now being a String instead of an Integer, is that a problem? We don't have bulbs connected to our boards yet. Can you check this change against a working Cobra build?

[cobra18t]

Have you tested with hardware connected? I know you don't have lights connected, but are the processors initializing as expected?

I don't have an STM32-based system that uses Incand wings. Maybe I can get one going.

I guess one concern would be that you initialize the OPPIncand class with the 3-part number while the OPPIncandCard class is initialized with the short number. Maybe this is fine, but it seems funny.

[unRARed]

We couldn't pass the duplicate light check, so I took the boards home to debug in a cozier environment =). So yea, no hardware connected right now. Seems like it's going to work? Anyway, I'll be bringing the board chain back to our shop probably next week (we just got snow here) and we can try hooking up some lamps used in the attract shows.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[unRARed]

As far as I can tell, this is working. We wired up a strand of bulbs from 0-0-0 to 0-0-9 which referenced in this show from the attract show.

Here's a clip of booting the game from this branch and displaying the working light show:

opp-incand.mov

Now, we do get a seemingly unrelated error when we press "start", due to what looks like OPP trying to communicate with the flippers (which we don't have hooked up and probably won't until we get the playfield art done). This is the code it tries to run. I assume we won't hit that once we have the coils wired, but thought it was worth mentioning.

[cobra18t]

Have you confirmed that assigning 0-0-0 and 0-1-0 or similar work with the hardware? I just want to see that the original issue with multiple cards has been eliminated in hardware.

I have created an STM32 setup to test this but have only made it to the stage where I can confirm the original MPF code works fine. I have not yet tried your changes to see if they break anything for the STM32 implementation.

[unRARed]

We only work on the physical game every few days or so. Next time will probably be Tuesday. My guess is most folks have simply not used incandescents the way we are across multiple Cypress boards. Maybe they only used 32 or less and had them all on a single board. That or they mixed in Fadecandy or some other RGB led solution? Or only used the incandescents for the gi?

This change is a single line of code, so we can use a fork from our end for now. I strongly believe there's nothing wrong with the functional code, and it's just the value being passed to the silly dup validator that's missing some context... ironically. We'll report back when we wire up another strand on from another board.

[cobra18t]

I confirmed with hardware that this does not impact the operation of the STM32 version of OPP. Looks good.

[unRARed]

Still can't definitively say since I'm not sure which exact bulb was hooked up on the chain, but we're definitely getting communication across boards.

opp-lights-dup.mov

The placeholder show we added.

[cobra18t]

Looks good. Glad you got it tested.

[cobra18t]

Sorry I forgot to approve this sooner.