jeelabs / jeelib

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

RF12_COMPAT mode broken since Jun 1 checkin #84

Open cmoore42 opened 9 years ago

cmoore42 commented 9 years ago

The checkin on Jun 1 (adapted changes from fixInterrupts branch) broke the RF12_COMPAT mode. Turning on RF12_COMPAT in RF12.h results in a compile failure in RF69.c:

...\jeelib\RF69.cpp: In function 'uint16_t RF69::recvDone_compat(uint8_t*)': ...\jeelib\RF69.cpp:171:27: error: lvalue required as left operand of assignment rxfill = rf12_len = 0; ^ But rf12_len is a #define in RF12.h, you can't assign 0 to it.

I'm not entirely clear why RF69.cpp is being compiled at all, but the only change I made was to set RF12_COMPAT to 1 and that caused the compile to fail.

JohnOH commented 9 years ago

if RF12_COMPAT

define rf12_len (rf12_buf[1] - 2)

extern volatile uint8_t rf12_buf[];

I think the attempted code line is: rxfill = (rf12_buf[1] - 2) = 0;

Which looks odd to me too.

Regarding compilation of RF69.cpp, I believe Arduino compiles all modules in a library into its temporary files directory and only the linker decides if they need to be pulled into the final image.

JohnOH commented 9 years ago

I take it that you are using #define RF12_COMPAT 0 to trigger the almost undocumented fixed packet length transport mechanism. Since use of the RFM12B radio is controlled by:

#define RF69_COMPAT      0   // define this to use the RF69 driver i.s.o. RF12 

before the jeelib include in the main sketch.

JohnOH commented 9 years ago

If at some point we look at the fixed packet length transport mechanism again I believe there is a much simpler run time option available as in: https://github.com/jcw/jeelib/commit/ffafa00c55c5f3a91b2ca036a80b52c7708a9a38

jcw commented 9 years ago

The problem comes from this change - line 164 / 171.

I'm inclined to change it back so it compiles, but it may not be the right thing to do for fixed-size packets. Probably better to break it and fix the most common case...

JohnOH commented 9 years ago

I did think up a run time fix for packets not needing the Jee header. There is a load of conditional stuff for the current compile time approach.