SpenceKonde / DxCore

Arduino core for AVR DA, DB, DD, EA and future DU-series parts - Microchip's latest and greatest AVRs. Library maintainers: Porting help and adviccee is available.
Other
181 stars 47 forks source link

OneWire does not work #426

Closed mikrocoder closed 1 year ago

mikrocoder commented 1 year ago

Hi,

did you change anything else on the OneWire or Pin Lib between Core 1.5.1 and 1.5.6? With v1.5.1 the address from the DS18B20 sensor is found and with v1.5.6. not. Nothing changes on any other pin.

Settings

//
//    FILE: oneWireSearch.ino
//  AUTHOR: Rob Tillaart
// VERSION: 0.1.02
// PURPOSE: scan for 1-Wire devices + code snippet generator
//    DATE: 2015-june-30
//     URL: http://forum.arduino.cc/index.php?topic=333923
//
// inspired by http://www.hacktronics.com/Tutorials/arduino-1-wire-address-finder.html
//
// Released to the public domain
//
// 0.1.00 initial version
// 0.1.01 first published version
// 0.1.02 small output changes

Stream &cout {Serial2};
#include <OneWire.h>

void setup()
{
  Serial2.swap(1);        // PF4 TXD2 / PF5 RXD2
  Serial2.begin(250000);
  delay(1);        
  cout.println("\nuC Reset ####");
  findDevices(PIN_PG2);
}

void loop()
{
}

uint8_t findDevices(int pin)
{
  OneWire ow(pin);

  uint8_t address[8];
  uint8_t count = 0;

  if (ow.search(address))
  {
    cout.print(pin, DEC);
    cout.print("[][8] = { ");
    do {
      count++;
      for (uint8_t i = 0; i < 8; i++)
      {
        cout.print("0x");
        if (address[i] < 0x10) cout.print("0");
        cout.print(address[i], HEX);
        if (i < 7) cout.print(", ");
      }
      cout.println(" };");
    } while (ow.search(address));

    cout.print("\ndevices found: ");
    cout.println(count);
  }

  return count;
}
SpenceKonde commented 1 year ago

Which version of OneWire are you using? Repo owner ignored my PR so only my fork works.

mikrocoder commented 1 year ago

I use the Dallas Lib from milesburton. https://github.com/milesburton/Arduino-Temperature-Control-Library Of the OneWire, there is one from Arduino and one from Paul Stoffregen. If I have the right overview. Do you still have your own (third) OneWire in the DxCore? I must first look more exactly what works here with whom. Thanks for the hint. It will take a while for me to find out ...

mikrocoder commented 1 year ago

Hello,

so I always have full backups of my portables. This helps immensely here.

In the IDE with DxCore 1.5.1 is the OneWire from Paul Stoffregen in version 2.3.5. In the IDE with DxCore 1.5.6 is the OneWire from Paul Stoffregen in version 2.3.7.

The OneWire versions differ in the file OneWire_direct_gpio.h

If I combine the OneWire 2.3.5 with DxCore 1.5.6 everything works.

What exactly is the problem with the OneWire? What did you want to change?

mikrocoder commented 1 year ago

Hi,

I have reduced the OneWire_direct_gpio.h of the v2.3.5 and v2.3.7 to AVR, so you can get the overview. I have the feeling the file of the v2.3.7 was completely rebuilt wrong.

// v.2.3.5
#pragma once

// This header should ONLY be included by OneWire.cpp.  These defines are
// meant to be private, used within OneWire.cpp, but not exposed to Arduino
// sketches or other libraries which may include OneWire.h.

#include <stdint.h>

// Platform specific I/O definitions

#if defined(__AVR__)
    #define PIN_TO_BITMASK(pin)             (digitalPinToBitMask(pin))
    #define IO_REG_TYPE uint8_t
    #define IO_REG_BASE_ATTR asm("r30")
    #define IO_REG_MASK_ATTR

    #if ((__AVR_ARCH__ == 102) || (__AVR_ARCH__ == 103) || (__AVR_ARCH__ == 104))
        #define PIN_TO_BASEREG(pin)             ((volatile uint8_t*)((digitalPinToPort(pin))<<2))
        #define DIRECT_READ(base, mask)         ((*((base)+2) & (mask)) ? 1 : 0)
        #define DIRECT_MODE_INPUT(base, mask)   ((*(base)) &= ~(mask))
        #define DIRECT_MODE_OUTPUT(base, mask)  ((*(base)) |= (mask))
        #define DIRECT_WRITE_LOW(base, mask)    ((*((base)+1)) &= ~(mask))
        #define DIRECT_WRITE_HIGH(base, mask)   ((*((base)+1)) |= (mask))
    #else
        #define PIN_TO_BASEREG(pin)             (portInputRegister(digitalPinToPort(pin)))
        #define DIRECT_READ(base, mask)         (((*(base)) & (mask)) ? 1 : 0)
        #define DIRECT_MODE_INPUT(base, mask)   ((*((base)+1)) &= ~(mask))
        #define DIRECT_MODE_OUTPUT(base, mask)  ((*((base)+1)) |= (mask))
        #define DIRECT_WRITE_LOW(base, mask)    ((*((base)+2)) &= ~(mask))
        #define DIRECT_WRITE_HIGH(base, mask)   ((*((base)+2)) |= (mask))
    #endif

    #ifndef PROGMEM
        #define PROGMEM
    #endif

    #ifndef pgm_read_byte
        #define pgm_read_byte(addr) (*(const uint8_t *)(addr))
    #endif

#endif
// v2.3.7
#pragma once

// This header should ONLY be included by OneWire.cpp.  These defines are
// meant to be private, used within OneWire.cpp, but not exposed to Arduino
// sketches or other libraries which may include OneWire.h.

#include <stdint.h>

// Platform specific I/O definitions

#if defined(__AVR__)
    #define PIN_TO_BASEREG(pin)                (portInputRegister(digitalPinToPort(pin)))
    #define PIN_TO_BITMASK(pin)                (digitalPinToBitMask(pin))
    #define IO_REG_TYPE uint8_t
    #define IO_REG_BASE_ATTR asm("r30")
    #define IO_REG_MASK_ATTR

    #if defined(__AVR_ATmega4809__)
        #define DIRECT_READ(base, mask)         (((*(base)) & (mask)) ? 1 : 0)
        #define DIRECT_MODE_INPUT(base, mask)   ((*((base)-8)) &= ~(mask))
        #define DIRECT_MODE_OUTPUT(base, mask)  ((*((base)-8)) |= (mask))
        #define DIRECT_WRITE_LOW(base, mask)    ((*((base)-4)) &= ~(mask))
        #define DIRECT_WRITE_HIGH(base, mask)   ((*((base)-4)) |= (mask))
    #else
        #define DIRECT_READ(base, mask)         (((*(base)) & (mask)) ? 1 : 0)
        #define DIRECT_MODE_INPUT(base, mask)   ((*((base)+1)) &= ~(mask))
        #define DIRECT_MODE_OUTPUT(base, mask)  ((*((base)+1)) |= (mask))
        #define DIRECT_WRITE_LOW(base, mask)    ((*((base)+2)) &= ~(mask))
        #define DIRECT_WRITE_HIGH(base, mask)   ((*((base)+2)) |= (mask))
    #endif

#endif
mikrocoder commented 1 year ago

Hello,

it looks like Paul has removed all new controllers and only considered the ATmega4809. This is of course stupid. I have pasted the lines of code from v2.3.5. and am currently testing with my AVR128DB64. Works so far. So that I keep the overview I have commented the lines. The ATmega4809 is addressed with normal PORT addresses. All others with the VPORT addresses. Because before the else branch did not fit to the Dx controllers this could not work. Let's see how to shorten this. The ATmega4809 should also use its VPORTs.

//#ifndef OneWire_Direct_GPIO_h  // v2.3.7
//#define OneWire_Direct_GPIO_h
#pragma once

// This header should ONLY be included by OneWire.cpp.  These defines are
// meant to be private, used within OneWire.cpp, but not exposed to Arduino
// sketches or other libraries which may include OneWire.h.

#include <stdint.h>

// Platform specific I/O definitions 

#if defined(__AVR__)
    #define PIN_TO_BITMASK(pin)                (digitalPinToBitMask(pin))
    #define IO_REG_TYPE uint8_t
    #define IO_REG_BASE_ATTR asm("r30")
    #define IO_REG_MASK_ATTR

    #if defined(__AVR_ATmega4809__)
        #define PIN_TO_BASEREG(pin)             (portInputRegister(digitalPinToPort(pin)))        // PORT.IN
        #define DIRECT_READ(base, mask)         (((*(base)) & (mask)) ? 1 : 0)                    // PORT.IN
        #define DIRECT_MODE_INPUT(base, mask)   ((*((base)-8)) &= ~(mask))                        // PORT.DIR
        #define DIRECT_MODE_OUTPUT(base, mask)  ((*((base)-8)) |=  (mask))                        // PORT.DIR
        #define DIRECT_WRITE_LOW(base, mask)    ((*((base)-4)) &= ~(mask))                        // PORT.OUT
        #define DIRECT_WRITE_HIGH(base, mask)   ((*((base)-4)) |=  (mask))                        // PORT.OUT

    #elif ((__AVR_ARCH__ == 102) || (__AVR_ARCH__ == 103) || (__AVR_ARCH__ == 104))
        #define PIN_TO_BASEREG(pin)             ((volatile uint8_t*)((digitalPinToPort(pin))<<2)) // VPORT Base = VPORT.DIR
        #define DIRECT_READ(base, mask)         ((*((base)+2) & (mask)) ? 1 : 0)                  // VPORT.IN
        #define DIRECT_MODE_INPUT(base, mask)   ((*(base))     &= ~(mask))                        // VPORT.DIR
        #define DIRECT_MODE_OUTPUT(base, mask)  ((*(base))     |=  (mask))                        // VPORT.DIR
        #define DIRECT_WRITE_LOW(base, mask)    ((*((base)+1)) &= ~(mask))                        // VPORT.OUT
        #define DIRECT_WRITE_HIGH(base, mask)   ((*((base)+1)) |=  (mask))                        // VPORT.OUT

    #else
        #define PIN_TO_BASEREG(pin)             (portInputRegister(digitalPinToPort(pin)))
        #define DIRECT_READ(base, mask)         (((*(base)) & (mask)) ? 1 : 0)
        #define DIRECT_MODE_INPUT(base, mask)   ((*((base)+1)) &= ~(mask))
        #define DIRECT_MODE_OUTPUT(base, mask)  ((*((base)+1)) |=  (mask))
        #define DIRECT_WRITE_LOW(base, mask)    ((*((base)+2)) &= ~(mask))
        #define DIRECT_WRITE_HIGH(base, mask)   ((*((base)+2)) |=  (mask))
    #endif

#endif
mikrocoder commented 1 year ago

Hi,

I have tried further today. The megaAVR0 is included in group 103. This means that the single query can be removed. What remains is ... I tested this with AVR128DB64 and ATmega4809 (Arduino Nano Every). Both work. What do you say? Okay or not okay? If you add the ESP lines again, everything is complete. Paul should have nothing to complain about.

//#ifndef OneWire_Direct_GPIO_h  // v2.3.7
//#define OneWire_Direct_GPIO_h
#pragma once

// This header should ONLY be included by OneWire.cpp.  These defines are
// meant to be private, used within OneWire.cpp, but not exposed to Arduino
// sketches or other libraries which may include OneWire.h.

#include <stdint.h>

// Platform specific I/O definitions 

#if defined(__AVR__)
    #define PIN_TO_BITMASK(pin)                (digitalPinToBitMask(pin))
    #define IO_REG_TYPE uint8_t
    #define IO_REG_BASE_ATTR asm("r30")
    #define IO_REG_MASK_ATTR

    #if ((__AVR_ARCH__ == 102) || (__AVR_ARCH__ == 103) || (__AVR_ARCH__ == 104))
        #define PIN_TO_BASEREG(pin)             ((volatile uint8_t*)((digitalPinToPort(pin))<<2)) // VPORT Base = VPORT.DIR
        #define DIRECT_READ(base, mask)         ((*((base)+2) & (mask)) ? 1 : 0)                  // VPORT.IN
        #define DIRECT_MODE_INPUT(base, mask)   ((*(base))     &= ~(mask))                        // VPORT.DIR
        #define DIRECT_MODE_OUTPUT(base, mask)  ((*(base))     |=  (mask))                        // VPORT.DIR
        #define DIRECT_WRITE_LOW(base, mask)    ((*((base)+1)) &= ~(mask))                        // VPORT.OUT
        #define DIRECT_WRITE_HIGH(base, mask)   ((*((base)+1)) |=  (mask))                        // VPORT.OUT
    #else
        #define PIN_TO_BASEREG(pin)             (portInputRegister(digitalPinToPort(pin)))
        #define DIRECT_READ(base, mask)         (((*(base)) & (mask)) ? 1 : 0)
        #define DIRECT_MODE_INPUT(base, mask)   ((*((base)+1)) &= ~(mask))
        #define DIRECT_MODE_OUTPUT(base, mask)  ((*((base)+1)) |=  (mask))
        #define DIRECT_WRITE_LOW(base, mask)    ((*((base)+2)) &= ~(mask))
        #define DIRECT_WRITE_HIGH(base, mask)   ((*((base)+2)) |=  (mask))
    #endif

#endif
SpenceKonde commented 1 year ago

That looks fine to me.

SpenceKonde commented 1 year ago

Paul's refusal to support new controllers (as if he has some sort of vested material interest in people using certain development boards or something) is why I have a fork of OneWire :-(

mikrocoder commented 1 year ago

Hi,

I made another inquiry with Paul. Otherwise, the only solution is your fork.

SpenceKonde commented 1 year ago

I think Paul S. is affilliated with / runs the show at Teensy, if that wasn't clear.

He may just have a chip on his shoulder with regard to modern AVRs though. A lot of people do. But they're just jealous because our tinyAVRs run circles around their ATmegas.

SpenceKonde commented 1 year ago

Closing as duplicate. This was reported and fixed and PR'ed in 2020, and the guy who controls the repo just doesn't want it to work on modern AVRs for some reason. This does not represent a defect in the core, nor is it an action item for me, so it should not be on the open issue list.