jeelabs / jeelib

JeeLib for Arduino IDE: Ports, RF12, and RF69 drivers from JeeLabs
https://jeelabs.org/202x/sw/jeelib/
The Unlicense
489 stars 215 forks source link

Update RF12.cpp #109

Open brandock opened 1 year ago

brandock commented 1 year ago

Update the pins for ATtiny84 to match the corrected pins in RF69_avr.h. Harmonizing the two makes sense. The scheme that has been in RF69_avr.h is the more modern, clockwise mapping in which MISO (PA6) is numbered 6, and SCK (PA4) is numbered 4.

brandock commented 1 year ago

Note that the pin mapping that has been working in RF69_avr.h has been there since 2015. https://github.com/jeelabs/jeelib/commit/21e26f25bffcd77669e26e8ecc2c494e7124b095

JohnOH commented 1 year ago

I was looking at this yesterday. May I ask which T84 core you are using. Does your change have implications for the JeeMicro hardware?

brandock commented 1 year ago

May I ask which T84 core you are using.

I started using Spence Konde's ATTinyCore this year. I also still have an old PC that I have kept around for the express purpose of running the arduino-tiny core, which requires an older version of the Arduino IDE (<=1.5) and various hacks to the toolchain that I can't even keep track of what they were after all these years.

As I moved to the newer ATTinyCore I hit a bump you can read about in the Arduino Forum. That is a separate issue, but my discussion with Spence Konde there illustrates one thing clearly: anyone who is using the older arduino-tiny core that is suggested on the JeeNode Micro page in the JeeLabs Shop is working very hard to maintain a museum-piece IDE that can't be readily upgraded to use newer tools.

Does your change have implications for the JeeMicro hardware?

It has implications, but it is difficult for me to say what they might be.

In my museum-piece PC that runs IDE 1.5x and arduino-tiny core, the same discrepancy between RFM69_avr.h and RF12.cpp SPI pins for ATtiny84 exists. So if I was working with the JeeNode Micro and switching between RFM69CW and RFM12B transceivers, I might get tripped up by the different SPI pin definitions in RF69_avr.h and RF12.cpp. I have rarely worked with the actual OG RFM12B, but if I did, I would not expect to have any luck until and unless I updated the pin definitions in RF12.cpp.

In my new world of working with a the modern Spence Konde ATTinyCore, I have no trouble using the RFM69CW transceiver because the pin definitions in RF69_avr.h match the default mapping for ATTinyCore, namely Clockwise. (NOTE: RF69_avr.h does not respect the user choice of mapping in ATTinyCore Arduino IDE menu options. It does not see the PINMAPPING_CW or PINMAPPING_CCW definitions at all. Which is probably worth an issue to address.) If I switch to the RFM12B receiver, I will succeed if I am using PINMAPPING_CCW with my sketch, because RF12.cpp uses the Counterclockwise pin mapping to define the SPI MUX. If I am using the RFM12B receiver and PINMAPPING_CW with my sketch, I expect I will fail because RF12.cpp does respect the PINMAPPING_CW definition, and the MISO and SCK pins will get reversed.

So, while there are implications, I think that they are generally to betterment of anyone who has been mostly working with the RFM69CW and grabs an old RFM12b device out of a drawer for some project, regardless of the core they are using. Anyone working with both transceivers would benefit from consistency.

JohnOH commented 1 year ago

You have been having fun. We did know about the active high on the RFM69 but never saw it as an issue on the T84 since we typically only use the T84 with the RFM12B. The concern I have is that Jeelib supports the existing hardware base from Jeelabs. If we adopted your change then existing users of the JeeMicro would have a problem since the software wouldn't match the hardware. I have been using ATTinycore 1.5.2 for a while along with Arduino 1.8.13 but have never linked a T84 with a RFM69CW. ATTinycore supports the pin mapping to be counter clockwise as a legacy option.

A paper about current T84 compile options and constraints would be a useful addition to the library.

brandock commented 1 year ago

I agree with the goal of preserving JeeLib's support of existing JeeLabs hardware. I am a fan.

My concern is that it is getting too hard for most users to maintain a toolchain that works with T84 and JeeLib. I consider myself at least somewhat above average in ability to work with these tools, and I don't think I could rebuild the arduino-tiny + Arduino 1.5x + needed mods toolchain even if I wanted to. I am fond of the T84 and I am not going back to that, nor would I suggest anyone use anything except ATTinyCore going forward.

I also think it is incorrect to assume that users will only use RFM12B with T84. Here is an article from 2014 where a JeeNode Micro is ordered with RFM69CW. I started working with JeeLib in 2015 and everyone was doing the "#define RF69_COMPAT 1" thing. I mean, JCW put it in there, so it is part of the ecosystem, IMO.

So I contend that JeeLib should work with both the original JeeLabs T84 hardware and the best modern core for T84 and both transceivers. I propose that JeeLib should check if ATTinyCore is in use and make sure that things are defined correctly for that core if so.

Here's my proposal for RF12.cpp:

#elif (defined(__AVR_ATtiny84__) || defined(__AVR_ATtiny44__)) && defined(PINMAPPING_CW) //ATTinyCore with Clockwise pin map 

#define RFM_IRQ     PIN_PB2
#define SS_DDR      DDRB
#define SS_PORT     PORTB
#define SS_BIT      1

#define SPI_SS      PIN_PB1     // PB1, pin 3
#define SPI_MISO    PIN_PA6     // PA6, pin 7
#define SPI_MOSI    PIN_PA5     // PA5, pin 8
#define SPI_SCK     PIN_PA4     // PA4, pin 9

#elif defined(__AVR_ATtiny84__) || defined(__AVR_ATtiny44__)

#define RFM_IRQ     2
#define SS_DDR      DDRB
#define SS_PORT     PORTB
#define SS_BIT      1

#define SPI_SS      1     // PB1, pin 3
#define SPI_MISO    4     // PA6, pin 7
#define SPI_MOSI    5     // PA5, pin 8
#define SPI_SCK     6     // PA4, pin 9

And in RF69.h I would propose adding:

#ifndef RF69_h
#define RF69_h

#if (defined(__AVR_ATtiny84__) || defined(__AVR_ATtiny44__)) && (defined(PINMAPPING_CW) || defined(PINMAPPING_CCW)) //ATTinyCore
  #undef SLEEP_MODE_STANDBY
#endif