issue #316 breaks interoperability with espressif/esp_io_expander

[cnf]

the patch applied due to #316 breaks interoperability with https://components.espressif.com/components/espressif/esp_io_expander/ as that only supports GPIO_MODE_INPUT, GPIO_MODE_OUTPUT, and GPIO_MODE_OUTPUT_OD

I could work around with with handleexternalpin, but then i lose the concurrency safety etc.

relevant code here:

esp_err_t esp_io_expander_gpio_wrapper_set_direction(gpio_num_t gpio_num, gpio_mode_t mode)
{
    esp_io_expander_handle_t handle = NULL;
    uint32_t pin_mask = 0;
    if (!find_ioexp_for_num((uint32_t)gpio_num, &handle, &pin_mask)) {
        ESP_LOGE(TAG, "GPIO %d is not assigned to any IO Expander", gpio_num);
        return ESP_ERR_INVALID_ARG;
    }
    esp_io_expander_dir_t dir;
    esp_io_expander_output_mode_t out_mode = IO_EXPANDER_OUTPUT_MODE_PUSH_PULL;
    bool mode_valid = true;
    switch (mode) {
    case GPIO_MODE_INPUT:
        dir = IO_EXPANDER_INPUT;
        break;
    case GPIO_MODE_OUTPUT:
        dir = IO_EXPANDER_OUTPUT;
        out_mode = IO_EXPANDER_OUTPUT_MODE_PUSH_PULL;
        break;
    case GPIO_MODE_OUTPUT_OD:
        dir = IO_EXPANDER_OUTPUT;
        out_mode = IO_EXPANDER_OUTPUT_MODE_OPEN_DRAIN;
        if (!handle->write_highz_reg) {
            mode_valid = false;
        }
        break;
    default:
        mode_valid = false;
    }
    if (!mode_valid) {
        ESP_LOGE(TAG, "Unsupported GPIO mode %d for IO Expander GPIO %d", mode, gpio_num);
        return ESP_ERR_INVALID_ARG;
    }
    esp_err_t err = esp_io_expander_set_dir(handle, pin_mask, dir);
    if (err == ESP_OK && dir == IO_EXPANDER_OUTPUT && handle->write_highz_reg) {
        err = esp_io_expander_set_output_mode(handle, pin_mask, out_mode);
    }
    return err;
}

https://github.com/espressif/esp-bsp/blob/eb76dc6ecf21ccc4ee7ee58bfea3d3d31fa090cf/components/io_expander/esp_io_expander/esp_io_expander_gpio_wrapper.c#L175-L213

[gin66]

Thanks for the info. Github actions are compile-testing against all supported esp32 variants, but none is using the esp-io-expander. Even though I am wondering, where this incompatibility comes from. The gpio supports it and then there is runtime-error or a compile time error ?

The original workaround avoids the readback, but would need IRAM_ATTR and access to the queue. This changes the semantics quite a lot.

The external pin was specifically added to support those extenders. From the esp-io-extenderdocmentation I could not figure out any assurance, that the pin has changed value on return and that it will take only „short“ time to be completed. Looking at the code of some of the drivers, these need to transmit some bytes via I2C. Not sure which bit rate. There is another peculiarity aka burden from 8bit world: All pin numbers need to be in range 0-63 for esp32: bit7=128 is used to flag external pin, and bit6=64 is used to flag i2s pins. Supporting esp32 gpio+io-expander comes close or exceeds 63….
Sure this can be changed, too, but I am not yet convinced to support esp-io-extender makes sense, just because espressif offers this functionality.

[cnf]

sorry for the late reply, i'm traveling atm.

Understood. what i was thinking is have the ability to set PIN_EXTERNAL_FLAG to GPIO_NUM_MAX for esp32 targets, as an option or by default?

the current "hack" i am using is the following"

bool pin_handler(uint8_t pin, uint8_t value) {
  gpio_num_t gpio_pin = to_expander_pin((gpio_num_t)pin);
  esp_err_t err = gpio_set_direction(gpio_pin, GPIO_MODE_OUTPUT);
  if (err != ESP_OK) {
    FLOG_ERROR("Failed to set GPIO direction for pin %d: %s", gpio_pin, esp_err_to_name(err));
    return !value;
  }
  err = gpio_set_level(gpio_pin, value);
  if (err != ESP_OK) {
    FLOG_ERROR("Failed to set GPIO level for pin %d: %s", gpio_pin, esp_err_to_name(err));
    return !value;
  }
  return bool(value);
}

// esp_io_expander pin  →  FastAccelStepper external pin
gpio_num_t to_fas_pin(gpio_num_t exp_pin) { return (gpio_num_t)(((int)exp_pin - GPIO_NUM_MAX) | PIN_EXTERNAL_FLAG); }

// FastAccelStepper external pin  →  esp_io_expander pin
gpio_num_t to_expander_pin(gpio_num_t fas_pin) { return (gpio_num_t)(GPIO_NUM_MAX + (fas_pin & ~PIN_EXTERNAL_FLAG)); }

and then initializing I do:

    _engine->setExternalCallForPin(pin_handler);
    _stepper->setDirectionPin(to_fas_pin(_port_config->dir_pin));
    _stepper->setEnablePin(to_fas_pin(_port_config->enable_pin));

the nice part of the io_expander framework is that other libraries etc that don't explicitly have callbacks for external handlers now mostly work.

Anyway, I think if we could set PIN_EXTERNAL_FLAG to GPIO_NUM_MAX in some way, the external callback can be used without the ugly mapping back and forth. that's a reasonably clean inbetween, isn't it?

Thanks for your time!

[gin66]

If exp_pin < 64, then the whole GPIO_NUM_MAX calculation can be omitted. As soon as the PIN_EXTERNAL_FLAG is set, the lower 6 bits are don't care. Most likely, if PIN_EXTERNAL_FLAGall seven bits are don't care. So the external pin need not to start with (0 | PIN_EXTERNAL_FLAG).

[cnf]

i'm sorry, i don't quite understand your last message...if the exp_pin is less than 64, which in my case it is, 51 and 54, FAA tries to use it as just a gpio pin. which doesn't work, because io_expander doesn't support GPIO_MODE_INPUT_OUTPUT

the pin nr needs to be higher than PIN_EXTERNAL_FLAG, right? to use the setExternalCallForPin external function?