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

OneWire malfunction on ESP32-S2 ? #122

Open
hervmele opened this issue Nov 8, 2022 · 22 comments
Open

OneWire malfunction on ESP32-S2 ? #122

hervmele opened this issue Nov 8, 2022 · 22 comments

Comments

@hervmele
Copy link

hervmele commented Nov 8, 2022

Description

I am currently working with ESP32-S2FN4R2 on a LOLIN S2 PICO board.

I am using OneWire library to connect DS18B20 (with Rob Tillaart DS18B20 library)... not very original !

I tried to use GPIO36 for Dallas chip connection ... but initial DS18B20::begin failed (false return)...

I tried to use another neighbor GPIO (35..38) with same result.

BUT, all seems ok with low numbered GPIO pins...

"begin" method call onWire search method to look for devices on "One Wire" bus.

I looked on library code and found that piece of code in OneWire_direct_gpio.h


static inline attribute((always_inline))
void directModeOutput(IO_REG_TYPE pin)
{
#if CONFIG_IDF_TARGET_ESP32C3
GPIO.enable_w1ts.val = ((uint32_t)1 << (pin));
#else
if ( digitalPinIsValid(pin) && pin <= 33 ) // pins above 33 can be only inputs
{
#if ESP_IDF_VERSION_MAJOR < 4 // IDF 3.x ESP32/PICO-D4
uint32_t rtc_reg(rtc_gpio_desc[pin].reg);

    if ( rtc_reg ) // RTC pins PULL settings

You can see that pin > 33 are excluded because "only inputs"...

It's true for ESP32, but not for ESP32-S2 (i don't know for ESP32-S3)...

I tried that little workaround...


if ( digitalPinIsValid(pin) /*&& pin <= 33*/ ) // pins above 33 can be only inputs

and my DS18B20 started talking on GPIO 36 !

So i think that OneWire Code should have special case for ESP32-S2 (may be S3 too)...

Sorry if i made mistake !

Steps To Reproduce Problem

Need ESP32S2 board and DS18B20 to be connected with.

Hardware & Software

Board LOLIN S2 PICO (or other S2 board i think !)
Shields / modules used
Arduino IDE version (i am working with PIO Core 6.1.5·Home 3.4.3)
Teensyduino version (if using Teensy)
Version info & package name (from Tools > Boards > Board Manager)... platformio ini file
; PlatformIO Project Configuration File
;
; Build options: build flags, source filter
; Upload options: custom upload port, speed and extra flags
; Library options: dependencies, extra library storages
; Advanced options: extra scripting
;
; Please visit documentation for the other options and examples
; https://docs.platformio.org/page/projectconf.html

[env:lolin_s2_pico]
platform = espressif32
board = lolin_s2_pico
framework = arduino
board_build.flash_mode = qio
lib_deps = 
paulstoffregen/OneWire@^2.3.7
robtillaart/DS18B20@^0.1.12

Operating system & version
Win 10 / VS Code / Platformio
Any other software or hardware?
Of course DS18B20 devices need to be connected on board 11 and 36 GPIO pin !

Arduino Sketch (PIO code)

#include <Arduino.h>
#include <OneWire.h>
#include <DS18B20.h>

OneWire oneWire36 (36); // DS18B20 on >33 GPIO
DS18B20 tempSensor36 (&oneWire36);

OneWire oneWire11 (11); // DS18B20 on low numbered GPIO
DS18B20 tempSensor11 (&oneWire11);

void setup ()
{
// Serial init and wait for terminal

Serial. begin (115200);

while (Serial. available () == 0)
yield ();

Serial. println ("go...");

if (tempSensor11. begin ())
Serial. println ("DS18B20 OK on GPIO11");
else
Serial. println ("DS18B20 KO on GPIO11");

if (tempSensor36. begin ())
Serial. println ("DS18B20 OK on GPIO36");
else
Serial. println ("DS18B20 KO on GPIO36");
}

void loop ()
{
}




@outdever
Copy link

outdever commented Dec 11, 2022

In ESP32-S3 I used it on pin 48, I also had to change lines like:
else if ( pin < 46 )
to
else
It was 135, 150, 163 lines

@jflasch2
Copy link
Contributor

Yep I ran into this also with a ESP32-S2 used the fix :

changed util/OneWire_direct_gpio.h line 240 to this and my ping 35 worked
if ( digitalPinIsValid(pin) /&& pin <= 33/ ) // pins above 33 can be only inputs

Can I make this change to the src ?

Thanks for your help .
Joe

@hervmele
Copy link
Author

Oh yes, I think so... The user needs to know which pin to use as input ! :-) Thanks.

@jflasch2
Copy link
Contributor

The datasheet for ESP32-S2 states that the only GPIO pin that is restrited to input only is GPIO46 and of course you don't want to connect to GPIO00 , so this code that turns GPIO33 and greater off is just wrong . This code should just generate a compile error if the pin is invalid and not what it does in disabling it from working with no error . The other problem is checking for all the ESP32 types to verify what pins you can and should not use would really complicated this code so lets not go there . So I'm going to propose just taking this check out for > 33 as a simple fix to not muck things up . So I'm goint to work on a pull requests to get this bad check fixed .

@uzi18
Copy link

uzi18 commented Aug 30, 2024

Just switch to onewireng and forget about it

@jflasch2
Copy link
Contributor

I'll have to try onewireng thanks .

@PaulStoffregen
Copy link
Owner

Please understand I do not use ESP32. I have a few boards with the older models, but none of the newer ESP32. So I am depending on the community to send good quality pull requests. The ESP32 code in this library now, which is causing a lot of complaints, was contributed by the community. I did not test it on any of the newer hardware, as I do not have any of that hardware.

@hervmele
Copy link
Author

hervmele commented Aug 31, 2024 via email

@jflasch2
Copy link
Contributor

Thanks for you work Paul, I'll be working on a simple Pull to fix this issue . I am reading up on the process .

@PaulStoffregen
Copy link
Owner

Thanks. A pull request is best. But if the tools are giving you trouble, I can work with just a copy of known-good code.

Again, I don't use ESP32 and the few pieces of hardware I have are the older models. The code we have today, which so many people are complaining about, was contributed from the community.

I'm really hoping you can send something which will satisfy all or most ESP32 users, or at least stem this tide of complaints. Long term, I need either better community contributed support for ESP32, or perhaps I might just pull all the ESP32 stuff and put an error message saying to use onewireng, if code that makes people happy isn't practical.

@uzi18
Copy link

uzi18 commented Aug 31, 2024

@PaulStoffregen also need to thank you for your work, have some original teensy > 3.0 and used already some of your libs.
About OneWireNg it is completely new adventure with 1wire with support for modern archs. Supports OverDrive mode and provide compatible OneWire class.
In fact it is faster to send people there and move along with project than complain about something doesn't work here.
ESP32 are now huge list of processors not completely compatible when talk about pins. Because of bugs by design (inside chip) some are only inputs some features was disabled by manufacturer later in SDK.
In my opinion they should have copy of this lib in core and support it there.

@jflasch2
Copy link
Contributor

It seems most of the issues about pins nums for esp32 seems to be limited to this idea that some pins should or shouldn't be used and thus the code is mucked up with checks for pin # 's . The real problem is code can't fix the problem of reading the doc for the chip to make sure you can use a pin for reading and writing like you need to for the OneWire . My feelings on this are no pin # 's should be checked for a validity check i.e. this is just wrong to muck up code trying to validate for all versions of chips out there because it will never end this checking . Thus a user puts a pin to use and is warn that the pin/gpio must be something that is read/writable in the doc for onewire ! The code we are fixing for this problem has to do with code that disables the function of pins > 33 within OneWire . The fix for this is a simple comment out this pin 33 check i.e. don't check for any pin numbers assume the user has seen the chip doc and the pin is read/writable .

@uzi18
Copy link

uzi18 commented Aug 31, 2024

@jflasch2 in fact this is left for user in OneWireNg to know if output mode of pin works or not.
So here you need only to check if pin is valid.
Usually users checks pinouts images and didn't know about datasheets exists ;)

@jflasch2
Copy link
Contributor

jflasch2 commented Sep 1, 2024

@uzi18 , It almost a wast of time to NOT pull the spec sheet today, pins have sometimes 5-7 functions and they are only going by a pinout ? Software can't help you to learn stuff from a spec sheet . I don't think there helping anyone with checks like that , sorry I disagree .

@uzi18
Copy link

uzi18 commented Sep 1, 2024

In my daily experience with arduino kiddies it looks like that - they just search for "board_name arduino pinout" and use it.
Sure it is workaround.

@PaulStoffregen
Copy link
Owner

I just want the pull request that will address this continuing stream of complaints.

If nobody can do that, I am seriously considering removing all ESP32 support and replacing it an error message directing ESP32 users to install onewireng.

@uzi18
Copy link

uzi18 commented Sep 1, 2024

@PaulStoffregen this is discussion outside PR ;)

@jflasch2
Copy link
Contributor

jflasch2 commented Sep 2, 2024

OK, how do I do a fork to make a pull requests ? I have cloned the repository local and now and I assume I have no permissions to push my changes to the master so now what ? The pull requests doc talks about doing a fork in the current repository etc , do I push this up to my repository or do a fork somewhere , I have no clue . The doc on this really sucks.

I have only done git stuff with my own push pull etc but never to another ?

@PaulStoffregen
Copy link
Owner

OK, how do I do a fork to make a pull requests ?

1: Create a fork. Look for the "Fork" button near upper right corner on main OneWire page.
2: Change the code on your fork. Only edit the stuff inside ifdefs for ESP32.
3: Carefully test your changes to make sure they really work on all ESP32 boards.
4: Look for the button to compare your changes with original. You'll find a button to send your changes as a pull request.

There are many other ways with various tools, but this way using the github website is the simpleste.

@jflasch2
Copy link
Contributor

jflasch2 commented Sep 4, 2024

Added pull requests , tested with pins 35 others have tested this simple change with other >= 33 pins . This disables the check which then allows valid ESP32 gpio to work >= 33 .

@PaulStoffregen
Copy link
Owner

The pull request was merged.

I'd like to ask everyone who has commented on this issue to please download and run the latest code. Does it work on your ESP32 board(s)?

@jflasch2
Copy link
Contributor

jflasch2 commented Sep 4, 2024

OK tested with my fix and it works for pin 35 . There is a problem in PIO in installing packages in that it will not pull a package if the version # doesn't change i.e. it uses a cashed version and thus my change wasn't reflected in 2.3.8 (In PIO) because pio is using some cashed version (?) and not the git hub version of 2.3.8 which currently does reflects my change . So to force pio to use the latest git hub you may have to increase the version to something like 2.3.9 . I manual changed the cashed pio version of 2.3.8 to the current 2.3.8 and tested with that and it works as I reported above . So for pio you would need to increment the version to ~2.3.9 to get the latest version in github and not the cashed pio version ? I know PIO can be a pain it seems .

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

No branches or pull requests

5 participants