Moddable-OpenSource / moddable

Tools for developers to create truly open IoT products using standard JavaScript on low cost microcontrollers.
http://www.moddable.com
1.35k stars 238 forks source link

Support both / two SPI hosts with different pin configurations #1243

Open ralphwetzel opened 1 year ago

ralphwetzel commented 1 year ago

Build environment: macOS Moddable SDK version: 4.3.0-9-gbbd413376 Target device: ESP32-2432S028R aka ESP32-CYD

Description ESP32-CYD looks like a "typical" development board. ESP32, a display driven by ILI9341, the XPT2046 as touch controller. It yet seems to be unique regarding the way of the integration of this touch controller: Whereas on other boards both devices - display & touch controller - are connected as (two) slaves to the same (single) SPI host, ESP32-CYD uses two SPI hosts to connect the devices. Both these hosts of course use their own (different) PIN configuration - the standard one (12, 13, 14) assigned to the display, (39, 32, 25) used for the touch controller.

As it looks like, the current implementation of the SPI bus in the SDK (modSPI) seems to be unable to support such a hardware design:

https://github.com/Moddable-OpenSource/moddable/blob/bbd413376dad348e44cb347377c3757cd9793993/modules/pins/spi/esp32/modSPI.c#L166-L168

https://github.com/Moddable-OpenSource/moddable/blob/bbd413376dad348e44cb347377c3757cd9793993/modules/pins/spi/esp32/modSPI.c#L152-L160

Consequentially - for ESP32-CYD - there's either the display operational, or the touch controller - but (currently) never both together.

Expected behavior Support hardware with two operational SPI busses at the same time.

Remark It would be very convenient to define the reqested SPI config as part of the device config, e.g.:

{
    defines: {
        [...],
        "xpt2046": {
        "hz": 1000000,
        "cs_pin": 33,
            [...],
            "spi": {
                "port": "SPI2_HOST",
                "miso_pin": 39,
                "mosi_pin": 32,
                "clock_pin": 25
            }
        }
    }
}

Those parameters could then be transferred into spiConfig in xs_XPT2046 @ modXpt2046.c, e.g.

void xs_XPT2046(xsMachine *the)
{
...
    modSPIConfig(xpt->spiConfig, MODDEF_XPT2046_HZ, MODDEF_XPT2046_SPI_PORT,
        MODDEF_XPT2046_CS_PORT, MODDEF_XPT2046_CS_PIN, xpt2046ChipSelect);

#ifdef MODDEF_XPT2046_SPI_MOSI_PIN
    xpt->spiConfig.mosi_pin = MODDEF_XPT2046_SPI_MOSI_PIN;
#endif
...

With modSPI being able to manage a second bus, the expected behavior would have been achieved.

phoddie commented 1 year ago

@ralphwetzel – yes, modSPI currently supports only a single SPI bus.

The overall structure of modSPI is similar to modI2C and modI2C supports multiple I2C busses. In theory, something similar could be done. That said, it isn't a small project.

FWIW... the code does seem to allow the SPI pins to be specified at runtime. This tricky bit of code does that: If miso_pin is 254, the pin number from the manifest is used, it is 255 the MISO pin is unused (-1), and otherwise it uses the specified PIN number. The ECMA-419 SPI for ESP8266 and ESP32 uses that.

 buscfg.miso_io_num = (254 == config->miso_pin) ? MODDEF_SPI_MISO_PIN : ((255 == config->miso_pin) ? -1 : config->miso_pin); 

That doesn't get to support for a second SPI bus, but it is one less problem to solve.

ralphwetzel commented 1 year ago

FWIW... the code does seem to allow the SPI pins to be specified at runtime.

Yes, it does - in a way. The current implementation yet initializes per default config->miso_pin = 254 @ modSPI.h. Thus the line you highlighted is used to pull in MODDEF_SPI_MISO_PIN from the manifest - and MODDEF_SPI_MISO_PIN is (part of) just a single set of pin definition.

To compensate, it could be an option to pull in the alternative pin set via (e.g.)

    #ifdef MODDEF_XPT2046_SPI_MISO_PIN
        xpt->spiConfig.miso_pin = MODDEF_XPT2046_SPI_MISO_PIN;
    #endif

when building the SPI config in modXpt2046.c.

That said, it isn't a small project.

Let me know, if/how I can support. The CYD got some attention in the NR forum and people seem to be interested in giving node-red-mcu a try...