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

BSP-276: Fix endianness and variable sizes (BSP-364) #202

Conversation

hephaisto
Copy link
Contributor

@hephaisto hephaisto commented Aug 5, 2023

ESP-BSP Pull Request checklist

Note: For new BSPs create a PR with this link.

  • Version of modified component bumped
  • CI passing

Change description

Fixes noted in #174

@hephaisto hephaisto mentioned this pull request Aug 5, 2023
2 tasks
@github-actions github-actions bot changed the title BSP-276: Fix endianness and variable sizes BSP-276: Fix endianness and variable sizes (BSP-364) Aug 5, 2023
Copy link
Collaborator

@espzav espzav left a comment

Choose a reason for hiding this comment

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

@hephaisto Thank you for fix this error. I haven't got HW for test it, it looks good.

@hephaisto
Copy link
Contributor Author

I haven't tested the fix on actual hardware yet. I currently do not have access to the chips ATM. If you feel more comfortable, I can try that next weekend or the one after.

@espzav
Copy link
Collaborator

espzav commented Aug 15, 2023

I haven't tested the fix on actual hardware yet. I currently do not have access to the chips ATM. If you feel more comfortable, I can try that next weekend or the one after.

Ok, it will be better to wait for try on real HW. I don't like make changes without try it. I will merge it, when you inform me, that it is working properly. Thank you very much!

@hephaisto
Copy link
Contributor Author

Tested on actual hardware using the following code:


#include <array>

#include <esp_io_expander_tca95xx_16bit.h>
#include <esp_log.h>

struct TestCase {
  uint32_t expanderPin;
  gpio_num_t gpioPin;
};

/*
 * This code expects a TCA9539 iwth the following connections:
 * TCA - ESP
 * ----------
 * A0  - GND
 * A1  - GND
 * SDA - IO23
 * SDC - IO22
 * IO0 - IO19
 * IO1 - IO18
 * IO7 - IO5
 */
const int sda = 23;
const int scl = 22;
const uint32_t address = ESP_IO_EXPANDER_I2C_TCA9539_ADDRESS_00;

std::array<TestCase, 3> testCases{{
    {IO_EXPANDER_PIN_NUM_0, GPIO_NUM_19},
    {IO_EXPANDER_PIN_NUM_1, GPIO_NUM_18},
    {IO_EXPANDER_PIN_NUM_7, GPIO_NUM_5},
}};

const char *logTag = "TCATEST";

class Runner {
public:
  void runTestCase(const TestCase &testCase) {
    ESP_LOGI(logTag, "running test case GPIO%i - TCA.%lu", testCase.gpioPin,
             testCase.expanderPin);
    currentTestCase = testCase;

    testInput(false);
    testInput(true);
    testOutput(false);
    testOutput(true);
    setTcaInput();
    setGpioInput();

    ESP_LOGI(logTag, "test case PASSED");
  }

  Runner() {
    ESP_LOGI(logTag, "initializing expander");
    i2c_config_t config{.mode = I2C_MODE_MASTER,
                        .sda_io_num = sda,
                        .scl_io_num = scl,
                        .sda_pullup_en = GPIO_PULLUP_ENABLE,
                        .scl_pullup_en = GPIO_PULLUP_ENABLE,
                        .master{
                            .clk_speed = 400'000,
                        },
                        .clk_flags = 0};

    ESP_ERROR_CHECK(i2c_param_config(i2cPort, &config));
    ESP_ERROR_CHECK(i2c_driver_install(i2cPort, I2C_MODE_MASTER, 0, 0, 0));

    ESP_ERROR_CHECK(
        esp_io_expander_new_i2c_tca95xx_16bit(i2cPort, address, &expander));
    ESP_LOGI(logTag, "expander initialized");
  }

private:
  const i2c_port_t i2cPort = 1;
  esp_io_expander_handle_t expander = nullptr;

  TestCase currentTestCase;

  void setTcaInput() {
    ESP_ERROR_CHECK(esp_io_expander_set_dir(
        expander, currentTestCase.expanderPin, IO_EXPANDER_INPUT));
  }
  void setTcaOutput() {
    ESP_ERROR_CHECK(esp_io_expander_set_dir(
        expander, currentTestCase.expanderPin, IO_EXPANDER_OUTPUT));
  }

  void setTcaOutputLevel(bool high) {
    ESP_ERROR_CHECK(esp_io_expander_set_level(
        expander, currentTestCase.expanderPin, high ? 1 : 0));
  }

  bool getTcaInputLevel() {
    uint32_t level;
    ESP_ERROR_CHECK(esp_io_expander_get_level(
        expander, currentTestCase.expanderPin, &level));
    return (level & currentTestCase.expanderPin) != 0;
  }

  void setGpioOutput() {
    ESP_ERROR_CHECK(
        gpio_set_direction(currentTestCase.gpioPin, GPIO_MODE_OUTPUT));
  }
  void setGpioInput() {
    ESP_ERROR_CHECK(
        gpio_set_direction(currentTestCase.gpioPin, GPIO_MODE_INPUT));
  }
  void setGpioLevel(bool high) {
    ESP_ERROR_CHECK(gpio_set_level(currentTestCase.gpioPin, high ? 1 : 0));
  }
  bool getGpioLevel() { return gpio_get_level(currentTestCase.gpioPin) != 0; }

  void testInput(bool high) {
    ESP_LOGI(logTag, "test input high=%u", high);
    setTcaInput();
    setGpioOutput();
    setGpioLevel(high);
    assert(getTcaInputLevel() == high);
  }

  void testOutput(bool high) {
    ESP_LOGI(logTag, "test output high=%u", high);
    setGpioInput();
    setTcaOutput();
    setTcaOutputLevel(high);
    assert(getGpioLevel() == high);
  }
};

extern "C" {
void app_main(void) {
  Runner runner;
  for (const auto &testCase : testCases)
    runner.runTestCase(testCase);
  ESP_LOGI(logTag, "ALL TESTS PASSED");
}
}

@hephaisto
Copy link
Contributor Author

I also noticed that no dependency on esp_io_expander was declared. I'm not very familiar with the ESP component system, so I am unsure if this is needed when it is added via managed components.

@espzav
Copy link
Collaborator

espzav commented Aug 28, 2023

Thank you for HW test!

I also noticed that no dependency on esp_io_expander was declared. I'm not very familiar with the ESP component system, so I am unsure if this is needed when it is added via managed components.

It is not neccessary, when you added via managed components. Please remove it.

@hephaisto hephaisto force-pushed the feature/BSP-276-add_io_expander_tca95xx_16_bit_fix_endianness branch from 615160a to 3a348ee Compare August 28, 2023 10:49
@hephaisto
Copy link
Contributor Author

It is not neccessary, when you added via managed components. Please remove it.

Removed.

@espzav espzav merged commit 0771193 into espressif:master Aug 28, 2023
19 checks passed
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.

2 participants