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

Possible fix for slow pin mode on Arduino >= 2.0.3 #3998

Closed
wants to merge 1 commit into from

Conversation

jenscski
Copy link
Contributor

@jenscski jenscski commented Nov 6, 2022

What does this implement/fix?

The GPIO implementation changed in Arduino 2.0.3, resulting in a slow pin mode change, resulting in dallas component stop working.

The PR changes to always use idf gpio handling i ESPHome regardless of compiling using IDF or using Arduino.

esphome/issues#3401
espressif/arduino-esp32#7049

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

Related issue or feature (if applicable): fixes

Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040

Example entry for config.yaml:

# Example config.yaml

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

if CORE.using_arduino:
return cv.declare_id(ArduinoInternalGPIOPin)(value)
raise NotImplementedError
return cv.declare_id(IDFInternalGPIOPin)(value)
Copy link
Member

Choose a reason for hiding this comment

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

Might as well remove the function entirely.

@@ -1,6 +1,5 @@
#pragma once

#ifdef USE_ESP32_FRAMEWORK_ESP_IDF
#include "esphome/core/hal.h"
#include <driver/gpio.h>
Copy link
Member

Choose a reason for hiding this comment

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

Did this compile for you? I think when compiling with Arduino, the IDF headers aren't available (at least by default).

Copy link
Member

@jesserockz jesserockz Nov 6, 2022

Choose a reason for hiding this comment

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

They are available and work, the issue here is that this file still needs #ifdef USE_ESP32 as the failure comes from the esp8266 test

@jesserockz
Copy link
Member

Haha, I actually just did the same change in #3564 (3f9deb1) and did not even see this, or your comment on the PR

@jesserockz
Copy link
Member

This most important thing to check is that this works with the existing arduino version which uses esp-idf v3. If not then it will have to be merged as part of #3564 only

@jenscski
Copy link
Contributor Author

jenscski commented Nov 7, 2022

Closing this as it's already fixed in #3564

@jesserockz did you take a look at the different timings regarding idf and Arduino in the esp_one_wire.cpp?

#ifdef USE_ESP32_FRAMEWORK_ARDUINO
uint32_t timing_constant = 14;
#elif defined(USE_ESP32_FRAMEWORK_ESP_IDF)
uint32_t timing_constant = 12;
#else
uint32_t timing_constant = 14;
#endif

@jenscski jenscski closed this Nov 7, 2022
ESPHome Dev automation moved this from Needs Review to Cancelled Nov 7, 2022
@jesserockz
Copy link
Member

Oh I did not, I will fix that up too. Thanks

@jenscski jenscski deleted the slow_arduino_gpio branch November 7, 2022 20:13
@github-actions github-actions bot locked and limited conversation to collaborators Nov 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
ESPHome Dev
Cancelled
Development

Successfully merging this pull request may close these issues.

None yet

3 participants