SpenceKonde / megaTinyCore

Arduino core for the tinyAVR 0/1/2-series - Ones's digit 2,4,5,7 (pincount, 8,14,20,24), tens digit 0, 1, or 2 (featureset), preceded by flash in kb. Library maintainers: porting help available!
Other
554 stars 144 forks source link

OneWireHub doesn't work with tinymegacore #925

Closed RoganDawes closed 1 year ago

RoganDawes commented 1 year ago

Hi, I'm trying to compile the OneWireHub examples for an ATTiny412, and am running into a number of issues relating to use of constexpr vs a #define (as far as I can make out, I'm no expert C++ programmer!)

Dependency Graph
|-- OneWireHub @ 2.2.3
Building in release mode
Compiling .pio/build/ATtiny412/src/tiny_ds_ntc.ino.cpp.o
Compiling .pio/build/ATtiny412/lib1ec/OneWireHub/BAE910.cpp.o
Compiling .pio/build/ATtiny412/lib1ec/OneWireHub/DS18B20.cpp.o
Compiling .pio/build/ATtiny412/lib1ec/OneWireHub/DS2401.cpp.o
Compiling .pio/build/ATtiny412/lib1ec/OneWireHub/DS2405.cpp.o
Compiling .pio/build/ATtiny412/lib1ec/OneWireHub/DS2408.cpp.o
Compiling .pio/build/ATtiny412/lib1ec/OneWireHub/DS2413.cpp.o
Compiling .pio/build/ATtiny412/lib1ec/OneWireHub/DS2423.cpp.o
Compiling .pio/build/ATtiny412/lib1ec/OneWireHub/DS2430.cpp.o
Compiling .pio/build/ATtiny412/lib1ec/OneWireHub/DS2431.cpp.o
In file included from .pio/libdeps/ATtiny412/OneWireHub/src/OneWireItem.h:4:0,
                 from .pio/libdeps/ATtiny412/OneWireHub/src/DS18B20.h:11,
                 from .pio/libdeps/ATtiny412/OneWireHub/src/DS18B20.cpp:1:
.pio/libdeps/ATtiny412/OneWireHub/src/OneWireHub.h: In function 'constexpr timeOW_t operator""_us(long long unsigned int)':
.pio/libdeps/ATtiny412/OneWireHub/src/OneWireHub.h:11:56: error: call to non-constexpr function 'uint32_t microsecondsToClockCycles(uint32_t)'
     return timeOW_t(time_us * microsecondsToClockCycles(1) / VALUE_IPL); // note: microsecondsToClockCycles == speed in MHz....
                               ~~~~~~~~~~~~~~~~~~~~~~~~~^~~
In file included from /Users/rogan/workspace/esphome/ntctest/attiny/src/tiny_ds_ntc.ino:9:0:
.pio/libdeps/ATtiny412/OneWireHub/src/OneWireHub.h: In function 'constexpr timeOW_t operator""_us(long long unsigned int)':
.pio/libdeps/ATtiny412/OneWireHub/src/OneWireHub.h:11:56: error: call to non-constexpr function 'uint32_t microsecondsToClockCycles(uint32_t)'
     return timeOW_t(time_us * microsecondsToClockCycles(1) / VALUE_IPL); // note: microsecondsToClockCycles == speed in MHz....
                               ~~~~~~~~~~~~~~~~~~~~~~~~~^~~
In file included from .pio/libdeps/ATtiny412/OneWireHub/src/OneWireItem.h:4:0,
                 from .pio/libdeps/ATtiny412/OneWireHub/src/DS2401.h:8,
                 from .pio/libdeps/ATtiny412/OneWireHub/src/DS2401.cpp:1:
.pio/libdeps/ATtiny412/OneWireHub/src/OneWireHub.h: In function 'constexpr timeOW_t operator""_us(long long unsigned int)':
.pio/libdeps/ATtiny412/OneWireHub/src/OneWireHub.h:11:56: error: call to non-constexpr function 'uint32_t microsecondsToClockCycles(uint32_t)'
     return timeOW_t(time_us * microsecondsToClockCycles(1) / VALUE_IPL); // note: microsecondsToClockCycles == speed in MHz....
                               ~~~~~~~~~~~~~~~~~~~~~~~~~^~~

I've tried to add constexpr annotation to these lines in ~/.platformio/packages/framework-arduino-megaavr-megatinycore/cores/megatinycore/Arduino.h:

uint16_t clockCyclesPerMicrosecond();
uint32_t clockCyclesToMicroseconds(uint32_t cycles);
uint32_t microsecondsToClockCycles(uint32_t microseconds);

but then the error changes to:

Compiling .pio/build/ATtiny412/lib1ec/OneWireHub/DS2431.cpp.o
In file included from .pio/libdeps/ATtiny412/OneWireHub/src/OneWireItem.h:4:0,
                 from .pio/libdeps/ATtiny412/OneWireHub/src/DS2401.h:8,
                 from .pio/libdeps/ATtiny412/OneWireHub/src/DS2401.cpp:1:
.pio/libdeps/ATtiny412/OneWireHub/src/OneWireHub_config.h:27:59:   in constexpr expansion of 'operator""_us(5000)'
.pio/libdeps/ATtiny412/OneWireHub/src/OneWireHub.h:11:56: error: 'constexpr uint32_t microsecondsToClockCycles(uint32_t)' used before its definition
     return timeOW_t(time_us * microsecondsToClockCycles(1) / VALUE_IPL); // note: microsecondsToClockCycles == speed in MHz....
                               ~~~~~~~~~~~~~~~~~~~~~~~~~^~~
In file included from .pio/libdeps/ATtiny412/OneWireHub/src/OneWireItem.h:4:0,
                 from .pio/libdeps/ATtiny412/OneWireHub/src/BAE910.h:8,
                 from .pio/libdeps/ATtiny412/OneWireHub/src/BAE910.cpp:1:
.pio/libdeps/ATtiny412/OneWireHub/src/OneWireHub_config.h:27:59:   in constexpr expansion of 'operator""_us(5000)'
.pio/libdeps/ATtiny412/OneWireHub/src/OneWireHub.h:11:56: error: 'constexpr uint32_t microsecondsToClockCycles(uint32_t)' used before its definition
     return timeOW_t(time_us * microsecondsToClockCycles(1) / VALUE_IPL); // note: microsecondsToClockCycles == speed in MHz....
                               ~~~~~~~~~~~~~~~~~~~~~~~~~^~~

I'm honestly not sure where the definition is that I should be updating? From what I can make out, the #include for Arduino.h is happening in platform.h, which is included via OneWireHub.h. Surely that is the right place for the definition of the functions?

SpenceKonde commented 1 year ago

I looks to me like timeOW must take a constexpr.

microsecondsToClockCycles is an inlinable function on my cores or was.

I'm not sure why that was the case when it's a macro on the official core, since that makes the difference between it working in cases like that and not working. Grab the latest wiring.c and Arduino.h from the github repo. I.... don't think it will break anything to pull those in without other changes, or you can modify the files in your copy of the core - wiring.c loses the definitions for microsecondsToClockCycles and clockCyclesToMicroseconds and clockCyclesPerMicrosecond. and Arduino.h loses all reference to those three functions, and instead gets this set of defines

#define clockCyclesPerMicrosecond() ((uint16_t)(F_CPU / 1000000L))
#define clockCyclesToMicroseconds(a) ((uint32_t)((a) / clockCyclesPerMicrosecond()))
#define microsecondsToClockCycles(a) ((uint32_t)((a) * clockCyclesPerMicrosecond()))

As an aside, what the hell is that library doing calling microsecondsToClockCycles(1) instead of clockCyclesPerMicrosecond()?

SpenceKonde commented 1 year ago

Fixed with 5e40bbf394dfc87e726339a2050387cc794ae804 on mTC and dc001ca478b626c6c690323220d3f248e824a048 on DxC

SpenceKonde commented 1 year ago

Issue should be resolved with current github version, closing. (note, this is megaTinyCore, not tinyMegaCore.