search

baldram / ESP_VS1053_Library

A library for VS1053 MP3 Codec Breakout adapted for Espressif ESP8266 and ESP32 boards.
https://platformio.org/lib/show/1744/ESP_VS1053_Library
GNU General Public License v3.0
114 stars 36 forks source link

Add compatibility with non-esp platforms, focus on RP2040 #90

Open episource opened 2 years ago

episource commented 2 years ago

The current implementation uses ESP-specific SPI.write*-API. Universally available is only SPI.transfer* (see documentation).

This PR changes spi writes to use the universal API on non-ESP platforms. For ESP platforms the existing implementation is preserved.

Additionally the bit value _BV macro is defined if it is not available by default.

Finally logging macros are extended to consider arduino-pico framework.

Functionallity has been tested using a Raspberry Pico Board (RP2040), but this PR is not limited to this platform. The changes included herein will likely make this library compatible with other platforms as well.

baldram commented 2 years ago

Hi @episource, than you for contribution!

The changes included herein will likely make this library compatible with other platforms as well.

It's exciting that it went so far, and I'm happy it's applicable beyond the original scope. The original plan was to focus on ESP and PlatformIO, so I had never expected it to be available in Arduino lib indexes. (Btw. PlatformIO supports Raspberry Pi Pico). I believe it still has a lot of ESP specific things hardcoded, and it's tested mostly among hardware with Espressif chips. However, if it works with other hardware, that's exciting! But damn, Raspberry? 🤯 Impressive!

Looking at the PR code, I see technically no changes comparing to the original. It's always good to do a smoke test. I guess you had no chance to run it against ESP32 or 8266? I have currently no setup, but I will have to rearrange it due to other PR's too.

PS: Have you considered adding some details on hardware you use with this library to the discussion here: https://github.com/baldram/ESP_VS1053_Library/discussions/61 or this issue: https://github.com/baldram/ESP_VS1053_Library/issues/1? Wiring and maybe some picture, it would be very welcome and appreciated 😃

episource commented 2 years ago

I've posted in #1. I'm not able to test the lib using ESP32 or ESP8266 as I do not own any of these. If I didn't make a big mistake, the code path's for ESP should not have changed, though.

baldram commented 2 years ago

@episource Would you please have a look at conflict in src/VS1053.h? It might be related to your other PR that was merged.

episource commented 2 years ago

Conflict solved.

Dr-Dawg commented 2 years ago

@baldram Please note that the MIDI PR uses a bunch of SPI.write commands in void VS1053::sendMidiMessage So there might be a few more conflicts..

baldram commented 2 years ago

@Dr-Dawg Good point. Easy to solve, but conflicting. I need to dig out the equipment, set up and perform tests, let's do it this weekend. Then merge PR's one by one.

episource commented 1 year ago

Conflicts resolved. Rebased to current master.