orgua / OneWireHub

OneWire slave device emulator
GNU General Public License v3.0
343 stars 86 forks source link

Compile errors with new ATTiny chips #89

Open picsil opened 4 years ago

picsil commented 4 years ago

This code works great on an ATTiny85. However I need to use the newer ATTiny 1-series chips in my design. I am trying to use the ATTiny1614, but the code will not compile. Many errors, but most similar to this: `In file included from /Users/reidforrest/software/picsil-i2c-bridge/picsil-i2c-bridge.ino:22:0: /Users/reidforrest/Documents/Arduino/libraries/OneWireHub/src/OneWireHub.h: In function 'constexpr timeOW_t operator""_us(long long unsigned int)': /Users/reidforrest/Documents/Arduino/libraries/OneWireHub/src/OneWireHub.h:11:56: error: call to non-constexpr function 'long unsigned int microsecondsToClockCycles(long unsigned int)' return timeOW_t(time_us * microsecondsToClockCycles(1) / VALUE_IPL); // note: microsecondsToClockCycles == speed in MHz....


In file included from /Users/reidforrest/software/picsil-i2c-bridge/picsil-i2c-bridge.ino:22:0:
/Users/reidforrest/Documents/Arduino/libraries/OneWireHub/src/OneWireHub.h: In function 'constexpr timeOW_t timeUsToLoops(uint16_t)':
/Users/reidforrest/Documents/Arduino/libraries/OneWireHub/src/OneWireHub.h:19:48: error: call to non-constexpr function 'long unsigned int microsecondsToClockCycles(long unsigned int)'
     return (time_us * microsecondsToClockCycles(1) / VALUE_IPL); // note: microsecondsToClockCycles == speed in MHz....
                       ~~~~~~~~~~~~~~~~~~~~~~~~~^~~`

I can get the code to compile if I replace microsecondsToClockCycles(1) with a constant of 10 or 20 (MHz I can run the ATTiny1614 at) but it does not work. When the 1-Wire master sends a reset pulse (checked on scope) with duration within specs, OneWireHub does not respond to it at all. I get no further than that.

P.S. I am open to using another chip. My requirement is for both reliable 1-Wire slave emulation and I2C master. I have not been able to achieve the I2C master communication using an ATTiny85.
kiowadriver commented 1 year ago

@picsil Did you find a resolution for this or pivot to another emulator library? I would like to use the ATTiny214 but have the same issue as you describe above.

RoganDawes commented 1 year ago

Getting the same thing. I see that #102 suggests the following:

\Arduino15\packages\esp32\hardware\esp32\2.0.6\cores\esp32\Arduino.h
change
#define clockCyclesPerMicrosecond() ( (long int)getCpuFrequencyMhz() )
to
#define clockCyclesPerMicrosecond() ( F_CPU / 1000000L )

But I see that in framework-arduino-megaavr-megatinycore/cores/megatinycore/wiring.c, it is defined as:

inline uint16_t clockCyclesPerMicrosecond() {
  return ((F_CPU) / 1000000L);
}

My C++ is horrible, should that uint16_t perhaps be marked as constexpr somehow?

RoganDawes commented 1 year ago

My thanks to SpenceKonde, who provided a fix for this almost immediately! See https://github.com/SpenceKonde/megaTinyCore/issues/925

Having eliminated all of the Serial calls from the example, it even fits within the available 4kB of flash available in the ATtiny412!

SpenceKonde commented 1 year ago

As an aside, I'll note that because of a collossal blunder on my part I don't think the release that has the fix (and two other criticals), release 2.6.6, is available. In fact I forgot to release it before I disassembled PWM which I haven't finished reassembling (there's going to be a menu in future mTC releases that lets you choose the mapping for the PWM pins, so you can say more the second have of TCA0 onto port C on larger pincount parts, as those pins aren't very useful otherwise). But thee shoudl be a decent release of mTC soon

Personally I think that it's kinda silly to have macros like that at all but nobody asked me (is it that hard to do (F_CPU/1000000) - both are compile time constant and are optmized away to a single value. This is just a way to put a wrapper around it!) But that's far from the worst thing i've seen in the official core, and I'm the one responsible for breaking it by changing it from a #define to a always inline function (IIRC) but now changed it back to make things like this work

imdbere commented 1 month ago

Has anyone gotten it to work fully on the new ATTinys? I am using the ATtiny816 and it builds fine, but then I have a similar problem as @picsil in that the device just does not respond to the master's reset pulse.

RoganDawes commented 1 month ago

I'd check that the clock is running at the right speed. Enable the GPIO strobe, and use a logic analyser to check the interval between strobes is as expected. I had trouble with the attiny85 behaving exactly as you describe, because the clock fuses were not set correctly, resulting in a 2Hz signal taking 8 seconds to toggle.

imdbere commented 1 month ago

Thanks, I have already tested the clock and it all looks good, I think there is a problem with the DIRECTWRITE macros on these controllers, I cannot get the debug pin to do anything and using this macros manually to toggle a pin does not seem to do anything either.

imdbere commented 1 month ago

Can confirm that that is the issue, the offsets used in this macros don't work with the new chips

  #define PIN_TO_BASEREG(pin)            (portInputRegister(digitalPinToPort(pin)))
  #define PIN_TO_BITMASK(pin)            (digitalPinToBitMask(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))

Changing it to this fixes it

  #define PIN_TO_BASEREG(pin)            ((digitalPinToPort(pin) * 4))
  #define PIN_TO_BITMASK(pin)            (digitalPinToBitMask(pin))
  #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))

Obviously not a real solution, just trying to outline the problem