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

Add support for panels with FM6126A chipset. Copied from SmartMat… #52

Merged
merged 3 commits into from Sep 21, 2022
Merged

Conversation

colorama
Copy link

Not sure if anyone needs it, but I have a bunch of these panels and now they work. Yay.
The fix adds one function - copied from SmartMatrix. Have not tested with 'normal' panels - I don't have any.
Might need to make this into an 'option'.

Copy link
Contributor

@PaintYourDragon PaintYourDragon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I have Normal Panels here and could test, but first, some changes I would request:

  • Please declare the new function as static and include PM at the start, just a pedantic thing in the library to say “internal function.”
  • The reset function can go above the #if defined(ARDUINO) || defined(CIRCUITPY) block, since nothing in here is platform-dependent.
  • Plz run through clang-format to match the formatting favored by the continuous integration tools.
  • I realize this is copied over from the SmartMatrix library, so I’m not placing any blame here, but the implementation is pretty bloaty and what I might ask is if you could re-test later after I’ve given it a going-through (since I don’t have any FM6126A panels here).

If that’s all bothersome, no worries and we are grateful for the contribution nonetheless…another option then is to keep a fork with your changes as they are, which can track and keep up with changes in the mainline code.
Thanks!

@colorama
Copy link
Author

Perhaps #define for loops is not awesome - feel free to change ;-)

@colorama
Copy link
Author

Also... I obviously have no idea what this code does - things like hardcoded maxLeds and the various offsets, etc w/o comments are not great. Regrettably, I don't have time to investigate further.

Copy link
Contributor

@PaintYourDragon PaintYourDragon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on “classic” panel, seems OK! Thank you! Will merge this in a moment but NOT bump the release # just yet — will make some small changes of my own and check back with you that it still works on the FM6126A panels

@PaintYourDragon PaintYourDragon merged commit c141ffb into adafruit:master Sep 21, 2022
@PaintYourDragon
Copy link
Contributor

Howdy. OK, startup/reset is simplified a bit, code’s in main branch but NOT Arduino lib manager yet. If you could check on the FM panels you have, and confirm whether it still works there, I’ll bump the version # which should send it out. Thx!

@colorama
Copy link
Author

colorama commented Sep 22, 2022

Pulled master and verified with Arduino - FM panels and "classic". All good 👍🏻

Related question: tried to verify with circuit python, so I pulled master of circuitpython and tried building for matrIxportal_m4 with:
make BOARD=matrixportal_m4

After copying the firmware to the board (cp firmware.uf2 /Volumes/MATRIXPORTAL), the board reboots, but does not mount as CIRCUITPY.

Any pointers? Thanks!

@PaintYourDragon
Copy link
Contributor

Woot, thanks.

Not sure what’s up with CircuitPython. Is this an artifact of the matrix code changes, or does this happen even with the prior code?

@colorama
Copy link
Author

Yeah, that's with TOT code. I'll try pulling an earlier tag.

@PaintYourDragon
Copy link
Contributor

Possibly related: changes were recently made for ESP-IDF 5.0.

@colorama
Copy link
Author

Same difference with 7.3.3. I must be doing something wrong. Just following instructions here:
https://learn.adafruit.com/building-circuitpython/build-circuitpython

@PaintYourDragon
Copy link
Contributor

Probably not you. CircuitPython’s a bit beyond me, but you can try asking in the forums.

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.

None yet

2 participants