PaulStoffregen / OneWire

Library for Dallas/Maxim 1-Wire Chips
http://www.pjrc.com/teensy/td_libs_OneWire.html
601 stars 390 forks source link

OneWire malfunction on ESP32-S2 ? #122

Open hervmele opened 1 year ago

hervmele commented 1 year ago

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

include

include

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 commented 1 year ago

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 commented 2 months ago

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 commented 2 months ago

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

jflasch2 commented 2 months ago

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 commented 2 months ago

Just switch to onewireng and forget about it

jflasch2 commented 2 months ago

I'll have to try onewireng thanks .

PaulStoffregen commented 2 months ago

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 commented 2 months ago

No problem Paul... Thanks for your work !

De : Paul Stoffregen @.> À : PaulStoffregen/OneWire @.> Sujet : Re: [PaulStoffregen/OneWire] OneWire malfunction on ESP32-S2 ? (Issue #122) Date : 31/08/2024 03:25:33 Europe/Paris Copie à : hervmele @.>;    Author @.>

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.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

jflasch2 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

@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 commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

@PaulStoffregen this is discussion outside PR ;)

jflasch2 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 .