Hi Tom, What is the likelihood that the RFM69 library, be updated to support the new SPI API's ?Hi Kiwi!
My understanding (see thread) is that Arduino 1.5.8 includes the new SPI capability, and the ethernet libraries have been updated with the new SPI functions, so updating the RFM69 libraries would
http://forum.arduino.cc/index.php?topic=273369.0
I would like to setup a W5100 ethernet RFM69 gateway, on a single moteino. I don't mind contributing to the effort / testing anyway I can.
Kiwi
Kiwi,I would agree it doesn't seem like a big change, but it would make the library play nice with the Ethernet, and other such libraries that adopt the SPI library. However I think there are some gotcha's, the receiveDone() method is a bit unusual.
I'll get the 1.5.8 IDE and check out the new functions. In principal the beginTransaction() and endTransaction() methods should be a direct replacement for select() and unselect() although the referenced object is different. And we'll need to add a method call to setup the new SPI configuration settings, but that should be easy to fold in.
Tom
<...snip>I agree and think we should pursue this. The only question is when... I'm too tied up right now, but I'm willing to review anything you do if you want to start a fork.
I would agree it doesn't seem like a big change, but it would make the library play nice with the Ethernet, and other such libraries that adopt the SPI library. However I think there are some gotcha's, the receiveDone() method is a bit unusual.
The issue isn't with the RFM library (as Felix disable interrupts), it is with other libraries that may not do the same and risk being interrupted (while actively doing SPI) by and RFM69 interrupt.Yeah, this was the part I found confusing about the transaction proposal. It only masked its own interrupt, leaving the transactions interruptable by other SPI devices (unless I misunderstood the code I admittedly just skimmed).
I typically have the latest 1.0.x and 1.5.x Arduino applications installed, with suitable renaming of the Application Package (on OS X) I don't have an issue. The application package bundles all the components, except your own contributed libraries an code. Not sure how windows handles it.Cool! Thanks! I'll give it a try.
Kiwi
Yeah, this was the part I found confusing about the transaction proposal. It only masked its own interrupt, leaving the transactions interruptable by other SPI devices (unless I misunderstood the code I admittedly just skimmed).I haven't read the new SPI code, I assumed that whenever a beginTransaction() was called that interrupts would be masked for any/all other PINS that have been registered, to prevent the interrupt (from the other SPI device) occurring until after then endTransaction() is called.
Don't 'break your back on this. I had a major setback on my current project today and may now actually have some time to work on this. I'll know more tomorrow.Yeah, this was the part I found confusing about the transaction proposal. It only masked its own interrupt, leaving the transactions interruptable by other SPI devices (unless I misunderstood the code I admittedly just skimmed).I haven't read the new SPI code, I assumed that whenever a beginTransaction() was called that interrupts would be masked for any/all other PINS that have been registered, to prevent the interrupt (from the other SPI device) occurring until after then endTransaction() is called.
YES I agree time is a problem, If I get to it will fork (on GItHub) and post link back to this forum
Kiwi
... AND, it appears at first glance, that the library DOES collate all the interrupts into a single mask. I won't be surprised to see that they do mask on every 'transaction', whether it's 'your' interrupt or not. ...I haven't looked at the library but would expect ALL SPI registered interrupts would be masked on beginTransaction. Interrupts are non-predictable...
Hi Tom, I got my Moteinos a couple of days ago, soldered and connected to a standard ethernet shield, so in a position to move forward. Where are you up to?Kiwi,
I would agree with your statement about backward compatibility. Note spi.h defines #define SPI_HAS_TRANSACTION 1 - for exactly that purpose. In terms of code cleanup:
- Static Declarations is an isolated fix, it has no bearing on the functionality in the code, so YES agreed.
- Non Volatiles. While I agree in principle it is a functional change, if this is changed IMHO it may introduce subtle bugs, my preference would be to exclude this
- Deadlock. I don't know about this issue, but if the resolution involved SPI locking/unlocking then YES should be addressed.... AND, it appears at first glance, that the library DOES collate all the interrupts into a single mask. I won't be surprised to see that they do mask on every 'transaction', whether it's 'your' interrupt or not. ...I haven't looked at the library but would expect ALL SPI registered interrupts would be masked on beginTransaction. Interrupts are non-predictable...
Kiwi.
EDIT: I just did a basic update of the RFM library to add beginTransaction() support, and will leave it running overnight with a gateway sketch just dumping my local RF network traffic (360 messages/hour), will report back if its still running in the morning.
@Kiwi,
<...snip>
https://github.com/kiwisincebirth/RFM69.
The main change in the RFM library is in the select() and unselect() methods, with a further small change in the init() method, where there interrupt handler is attached.
There is a small fix in readAllRegs() method to remove an unnecessary (duplicated) unselect() method call.
Lastly in the receiveDone() method, a call the interrupts() method has been added before every return statement. This is to ensure interrupts are fully enabled before the function ends. Previously this was implicit via a sub-functon calling unselect(), but this cannot be relied upon in the amended unselect(). The new SPI libraries disable (mask) only the interrupts that it knows need to be masked, rather then globally disable all interrupts.
It is lastly this last change that needs the most review, as I am not across the reason for disabling interrupts globally in this way, with the new SPI libraries this may not even be needed.
<snip...>
@Kiwi,@Kiwi,
<...snip>
https://github.com/kiwisincebirth/RFM69.
The main change in the RFM library is in the select() and unselect() methods, with a further small change in the init() method, where there interrupt handler is attached.
There is a small fix in readAllRegs() method to remove an unnecessary (duplicated) unselect() method call.
Lastly in the receiveDone() method, a call the interrupts() method has been added before every return statement. This is to ensure interrupts are fully enabled before the function ends. Previously this was implicit via a sub-functon calling unselect(), but this cannot be relied upon in the amended unselect(). The new SPI libraries disable (mask) only the interrupts that it knows need to be masked, rather then globally disable all interrupts.
It is lastly this last change that needs the most review, as I am not across the reason for disabling interrupts globally in this way, with the new SPI libraries this may not even be needed.
<snip...>
I just donwloaded your updates and will review today. Re the global interrupts() call, ISTM this is unnecessary and, in the 'wrong' circumstances(ie a processor with more interrupt sources/levels), could create problems. It seems like it would be unnecessary since you do pair begin/endtransactions and this should keep the keep the proper interrupt levels unmasked (assuming the new SPI library does ;-)
Again, I'll get back to you later today.
Tom
Hi there Tom, I am using the standard arduino Ethernet shield with W5100 chipset. From a Moteino perspective this board chip are 3V3 compatible, so can be directly connected to the moteino pins.@Kiwi,
Good news, is my gateway test worked, 10 hours of operation without issue that I can see. Next step is to include some Ethernet running at same time.
Will post code for review tonight, most change is in select() unselect()
Kiwi
@Kiwi,My belief is the receive done method needs to be changed. Either the interrupts() removed, or balanced as I have done in GitHub
I looked over the code and everything looks good EXCEPT the changes in receiveDone(), which I'll get to in a moment.
The #ifdef approach works well without a lot of messiness. Good job and good approach in select().
Re receiveDone, the problem I see is that the 'transaction' is now more than just a SPI sort of thing. receiveDone() disables interrupts so that it can perform an atomic test and execute on the mode and payloadlen, but then relies on this to be 'fixed' at the end of the unselect() on either the setMode() or receiveBegin() calls. This MIGHT be ok, if the logic in the begin/endtransaction isn't state dependent, ie, that interrupts are already disabled. Like your counting approach, this could be a problem if the transaction logic simply restores the 'previous' state. I'm not sure the current version does, probably due to the simplicity of the processor interrupt logic, it just seems 'unbalanced' to me.
If we accept that the endtransaction will re-enable interrupts, then the comments in the code indicate that receiveBegin() will 'unselect' (and therefore re-enable interrupts), hence your addition here is not necessary.
What are your thoughts?
Tom
Your explanation is very clear. I understand your point. I'm hoping to have some time tomorrow to look at it and will reply once I have a sense of what is there (or more questions).
<...snip>My belief is the receive done method needs to be changed. Either the interrupts() removed, or balanced as I have done in GitHub
The reason it has to change is that the endtransaction() method in SpI library do not reenable interrupts. you can see more clearly what is happening in the begintransaction() spi method. This method is design to mask interrupts for the interrupt pins that have been registered via usinginterrupts(), but not disable interrupts entirely. This allows timer0 millis() to continue to function.
Thus if we rely on end transaction to reenable interrupts then this won't happen.
The approach that I have taken, assumes that the blocking of interrupts is necessary for the atomic locking to occur, if not it should be removed.
Since the disabling of interrupts and masking of interrupts i think are done on different registers there shouldn't be an issue. Also so long as the two mechanisms happen in matching pairs, don't overlap instead one surrounds the other we should be ok,
I.e. Spi end transaction is called and restores the interrupt state when begintransaction was called, then the receive done method enables the interrupts
Hope the makes sense.
Ps I am away from home for a couple of days, so can't check Ethernet board
@Kiwi,I have just read some forum posts, and delved into the Arduino library itself. And NO interrupts are not supported by the standard ethernet library, the bridging pin you refer to could be used ONLY with a redeveloped library.
is the W5100 shield you're using have the Interrupt jumper (a little solder pad) bridged or open? Do you know if the ethernet library is making the SPI.usingInterrupt() call (and which interrupt number it's using)?
Tom
Great! Thanks for looking into this. I should be able to work on the transaction conversion of RFM69 today.@Kiwi,I have just read some forum posts, and delved into the Arduino library itself. And NO interrupts are not supported by the standard ethernet library, the bridging pin you refer to could be used ONLY with a redeveloped library.
is the W5100 shield you're using have the Interrupt jumper (a little solder pad) bridged or open? Do you know if the ethernet library is making the SPI.usingInterrupt() call (and which interrupt number it's using)?
Tom
@Kiwi, I have to tell you that I scratched my head long and hard on this and, in the end, have come full circle. I think you're completely right about using noInterrupts/interrupts() in receiveDone().@Kiwi,My belief is the receive done method needs to be changed. Either the interrupts() removed, or balanced as I have done in GitHub
I looked over the code and everything looks good EXCEPT the changes in receiveDone(), which I'll get to in a moment.
The #ifdef approach works well without a lot of messiness. Good job and good approach in select().
Re receiveDone, the problem I see is that the 'transaction' is now more than just a SPI sort of thing. receiveDone() disables interrupts so that it can perform an atomic test and execute on the mode and payloadlen, but then relies on this to be 'fixed' at the end of the unselect() on either the setMode() or receiveBegin() calls. This MIGHT be ok, if the logic in the begin/endtransaction isn't state dependent, ie, that interrupts are already disabled. Like your counting approach, this could be a problem if the transaction logic simply restores the 'previous' state. I'm not sure the current version does, probably due to the simplicity of the processor interrupt logic, it just seems 'unbalanced' to me.
If we accept that the endtransaction will re-enable interrupts, then the comments in the code indicate that receiveBegin() will 'unselect' (and therefore re-enable interrupts), hence your addition here is not necessary.
What are your thoughts?
Tom
The reason it has to change is that the endtransaction() method in SpI library do not reenable interrupts. you can see more clearly what is happening in the begintransaction() spi method. This method is design to mask interrupts for the interrupt pins that have been registered via usinginterrupts(), but not disable interrupts entirely. This allows timer0 millis() to continue to function.
Thus if we rely on end transaction to reenable interrupts then this won't happen.
The approach that I have taken, assumes that the blocking of interrupts is necessary for the atomic locking to occur, if not it should be removed.
Since the disabling of interrupts and masking of interrupts i think are done on different registers there shouldn't be an issue. Also so long as the two mechanisms happen in matching pairs, don't overlap instead one surrounds the other we should be ok,
I.e. Spi end transaction is called and restores the interrupt state when begintransaction was called, then the receive done method enables the interrupts
Hope the makes sense.
Ps I am away from home for a couple of days, so can't check Ethernet board
...I have made the changes as you suggested, and created a pull request.
I really struggled with this because strictly speaking, receiveDone() should only need to be atomic with respect to its own interrupt, which, in this case, is the same as the SPI interrupt. Unfortunately there are two problems with this 'purist' view, the first being that a 'universal' receiveDone() would have to do the same work as SPI.usingInterrupt() to generate the appropriate mask and a quick look at the code needed to parse SPI.usingInterrupt() for all possible processors makes your head spin!
The second aspect is more a 'conservative' concern that it may not be JUST SPI interrupts we need to block in this case. It may be that ANY interrupts used by the Moteino application may need to be blocked at this point and noInterrupts() does just that. And, as you already concluded, since the setMode() and receiveBegin() don't restore global interrupt (if using SPI transactions) then the extra interrupts() calls are necessary in receiveDone(). However, since these are ONLY needed in the SPI transaction case, I think we should use a conditionally defined INTERRUPTS() (or conditionally execute interrupts()) in these two places.
Do you agree?
Once this is resolved, I think what you have is great and should be submitted to the 'committee' (AKA Felix) via pull request.
Tom
This is how:
void RFM69::setCS(uint8_t newSPISlaveSelect)
You are correct sir. You move one of the pins to something else :)I'm confused. The wiring from pin to the RFM module would be a copper trace on the Moteino PCB. I don't see how you would change anything.
Cut, resolder to another pin using thin hookup wire, add 1 line of code, done.ok, I see. I was thinking there might be a less invasive option.
ok, I see. I was thinking there might be a less invasive option.I have followed your approach and moved the Ethernet CS Pin to a different location, and downloaded the updated libraries you referenced.
<...snip>The motivation wasn't saving code, but an effort to try to keep the calls to noInterrupts()/interrupts() balanced. If these ever permit nested hierarchical calls, then an unbalance isn't a good thing. I'm with you on struggling with the dependency, but, as you say, one step at a time...
I question the motivation for excluding the extra line of code, All this to save a few extra bytes in flash, and clock cycles at runtime, I know this is a constrained platform, but really...
I come from a professional Java coding background, in a large scale system this sort of thing wouldn't be accepted because across many 10's or 100's of thousands of line it is impossible to understand manage.
Kiwi.
Hi kiwisincebirth!I have made this change. I am not sure why the the new SPI library doesn't do this in the begin/end transaction, I am guessing the assumption is that SPI will be configured before each access to SPI
As it is not done in the implementation of beginTransaction() (https://github.com/arduino/Arduino/blob/a9735bf91fda36eb9cd97274f01b86f1fb234155/hardware/arduino/avr/libraries/SPI/SPI.h#L178-L205) and endTransaction() (https://github.com/arduino/Arduino/blob/a9735bf91fda36eb9cd97274f01b86f1fb234155/hardware/arduino/avr/libraries/SPI/SPI.h#L260-L284) I would suggest placing the lines which save (https://github.com/kiwisincebirth/RFM69/blob/e707b9a3adfee9e9567bfc2288ba7734cd5d52ba/RFM69.cpp#L432-L434) and restore (https://github.com/kiwisincebirth/RFM69/blob/e707b9a3adfee9e9567bfc2288ba7734cd5d52ba/RFM69.cpp#L449-L451) the current SPI settings in front of the #ifdefs to not interfere with other code that accesses SPI and relies on a previous configuration.
You have a typo - "doest" (https://github.com/kiwisincebirth/RFM69/blob/e707b9a3adfee9e9567bfc2288ba7734cd5d52ba/RFM69.cpp#L358) but i don't know if you mean does or doesn't as beginTransaction() saves (https://github.com/arduino/Arduino/blob/a9735bf91fda36eb9cd97274f01b86f1fb234155/hardware/arduino/avr/libraries/SPI/SPI.h#L180) and endTransaction() restores (https://github.com/arduino/Arduino/blob/a9735bf91fda36eb9cd97274f01b86f1fb234155/hardware/arduino/avr/libraries/SPI/SPI.h#L281) the SREG and therefore the interrupt bit in SREG is the same as before an SPI access.I have updated the code to more correctly document the change in the receiveDone() method. However it is important to note that the interrupts are disabled in this method to gain an atomic lock in the code, not specifically about locking SPI access. It just so happened that the completion of SPI calls interrupts are re-enabled by unselect(), thus nointerrupts() didn't need to be called. But with the new SPI library this is not guaranteed to be true, hence noInterrupts() needs to be explicitly called.
In the comments it says that "New SPI Library doesn't disable interrupts" but they are enabled here (https://github.com/kiwisincebirth/RFM69/blob/e707b9a3adfee9e9567bfc2288ba7734cd5d52ba/RFM69.cpp#L363) and here (https://github.com/kiwisincebirth/RFM69/blob/e707b9a3adfee9e9567bfc2288ba7734cd5d52ba/RFM69.cpp#L374)?
And just delete that unnecessary unselect (https://github.com/kiwisincebirth/RFM69/blob/e707b9a3adfee9e9567bfc2288ba7734cd5d52ba/RFM69.cpp#L501) entirely.Done.
The motivation wasn't saving code, but an effort to try to keep the calls to noInterrupts()/interrupts() balanced. If these ever permit nested hierarchical calls, then an unbalance isn't a good thing. I'm with you on struggling with the dependency, but, as you say, one step at a time...To that end I have removed the #ifdefs out of my receiveDone(), and always call interrupts() to balance the noInterrupts() at the the top of this method. This archives matching calls, and removes the dependancy on the select() and unselect() entirely.
Thanks for your work. Hopefully I'll be able to join in the testing soon, right now I'm butt deep in my gateway code...
Thanks for the W5100 info. I've ordered some of the 'Mini Shields' (for my gateway) and will be able to test once the 'slow boat' arrives...
Tom
<...snip>Your comments make sense with respect to the already unbalanced nature of the noInterrupts()/interrupts() calls and I agree with the change. I can't look at the code right now so can't comment on the SREG discussion although I will say I'm concerned about the possible re-entrance of interruptHandler(). I'll try to get to it later today.The motivation wasn't saving code, but an effort to try to keep the calls to noInterrupts()/interrupts() balanced. If these ever permit nested hierarchical calls, then an unbalance isn't a good thing. I'm with you on struggling with the dependency, but, as you say, one step at a time...To that end I have removed the #ifdefs out of my receiveDone(), and always call interrupts() to balance the noInterrupts() at the the top of this method. This archives matching calls, and removes the dependancy on the select() and unselect() entirely.
Note: Previously it wasn't balanced i.e. recieveDone() calls noInterupts(); select() calls noInterrupts(); unselect calls interrupts(); so 2 x Off and 1 x On.
ALSO. I think the best improvement would be simply to save and restore SREG, where interrupts are enabled and disabled in the code. e.g. select() and unselect()
This would solve a potential problem in interruptHandler(). When interruptHandler() calls a method that requires SPI, that method calls select() does it work then calls unselect(), unselect() then calls interrupts(). Which I assume means for the remainder of the interruptHandler() interrupts can occur.
I am not sure if this is a bug, and easily fixed by saving and restoring SREG in select() unselect(), OR it could be a FEATURE, meaning that other interrupts can occur (e.g. Timer0) during the rest of the interrupt, I don't know how long interruptHandler() takes to execute.
<snip...>
Hi kiwisincebirth!I have corrected my branch, assume you can see this. Note: I simplified the code with a single #if compiler directive in each of the select() and unselect() methods. IMHO this make it more readable, since you can now clearly identify the two distinct blocks of code. The original code is now back in its unmodified (except fro SREG) state.
Your new commits are visible in the Pull Request.
Thank you for implementing my suggestions.
I have another suggestion regarding the current state.
https://github.com/damadmai/RFM69/commit/69620f280b5ba8eca991c107097f36a3d9d50d6b
As it might be possible that an interrupt occurs after saving the SPCR and SPSR I think it would be better if the call to noInterrupts() stays where it was like in select().
(and SREG needs only be saved in _SREG if we don't have SPI_HAS_TRANSACTION ;))
And as endTransaction doesn't change SPCR or SPSR unselect() could be written for the same reason as in my commit.
In the Header file it might be necessary to do some #ifdef
In this line with your changes the comment "enables interrupts" would not be true any more because unselect does not re-enable interrupts even if they were not enabled before!
https://github.com/LowPowerLab/RFM69/pull/25/files#diff-2d1040d890d26d4ac6877f9269b3e32fR367
So just please remove this comment.
Thats a comlex change. I just thought two hours about it...
Does anyone know if there is a better way on GitHub to suggest changes to an Pull Request like my manual approach? :)
Daniel
Daniel, I've added a comment to your commit. Net: I agree with your change and also added that I think a SPIsettings variable, _settings, should be conditionally added and initialized once, rather than calculate the value on each beginTransactions() call.
Tom
The new SPISettings is a special data type, just for describing SPI clock, data order and format. For fixed settings, you can use beginTransaction(SPISettings(clock, order, format)), and the compiler will automatically inline your fixed settings with the most optimal code. For user controlled settings, you can create a variable of SPISetting type and assign it based on user choice, which you don't know in advance. This allows a very efficient beginTransaction(), because the non-const settings are converted to an efficient form ahead of time.
This is an ultra major potentially breaking change and it's very labor intensive to test it properly.I can understand your caution.
I'm not sure when I will get to this.
Also I am not sure package deal SPI transactions are the best thing either, as we've seen with other things bundled with Arduino. SPI "transactions" were already implemented in the library in a raw way.
Stay tuned, a much more usable and more automatic interface for Moteino based stuff is coming from Low Power Lab. It will only require a few files.
Ah, very good. Thank you very much for pointing this out, I had not realized that - very clever indeed! I used a variable in the SPIFlash update, perhaps I should change it? On the other hand, I noticed that a variable was used in the IDE 1.6.0 SD library - maybe they don't trust the compiler as much as the writers of that comment?Daniel, I've added a comment to your commit. Net: I agree with your change and also added that I think a SPIsettings variable, _settings, should be conditionally added and initialized once, rather than calculate the value on each beginTransactions() call.
Tom
Hi Tom, From the Article
http://dorkbotpdx.org/blog/paul/spi_transactions_in_arduinoQuoteThe new SPISettings is a special data type, just for describing SPI clock, data order and format. For fixed settings, you can use beginTransaction(SPISettings(clock, order, format)), and the compiler will automatically inline your fixed settings with the most optimal code. For user controlled settings, you can create a variable of SPISetting type and assign it based on user choice, which you don't know in advance. This allows a very efficient beginTransaction(), because the non-const settings are converted to an efficient form ahead of time.
This statement in the article was the reason I put the SPISettings inside the call to beginTransaction()
Kiwi
Hello everyone, I'm interested in creating a W5100 + Moteino gateway. I've been reading through this post and am confused regarding the exact status. Which is the repository I should be using to try this out?Kiwi will have to advise you on the specifics on using the W5100 as I'm pretty sure he's using it in his Moteino Gateway.
Regards,
What I understood is stopping the RF69 and Ethernet libraries from working together happily is that they are both using pin 10 as SS. The fix would be to physically move the RF69 SS pin to another (9). And then change the SS using the line that I believe Felix posted previously. I would use 1.0.6 if 1.6.0 is causing problems.I believe that the SS pin for the W5100 can be moved to virtually any pin. Where it can't be moved is on an Arduino shield THAT IS plugged directly into an Arduino, but that's not an issue with Moteino - the Arduino shield will not plug directly into a Moteino board. Consequently, whether you use the shield or a W5100 breakout board or design your own PCB, you can connect a different pin from the Moteino to the W5100 - this is a HARDWARE statement.
Am I understanding this correctly or missing something vital? I have searched the forum but haven't found mention of an Ethernet/Moteino Gateway apart from this thread.
Regards,
/*
DHCP-based IP printer
This sketch uses the DHCP extensions to the Ethernet library
to get an IP address via DHCP and print the address obtained.
using an Arduino Wiznet Ethernet shield.
Circuit:
* Ethernet shield attached to pins 10, 11, 12, 13
created 12 April 2011
modified 9 Apr 2012
by Tom Igoe
*/
#include <SPI.h>
#include <Ethernet.h>
#include <utility/w5100.h>
// Enter a MAC address for your controller below.
// Newer Ethernet shields have a MAC address printed on a sticker on the shield
byte mac[] = {
0x00, 0xAA, 0xBB, 0xCC, 0xDE, 0x33
};
// Initialize the Ethernet client library
// with the IP address and port of the server
// that you want to connect to (port 80 is default for HTTP):
EthernetClient client;
void setup() {
W5100.select(8);
// Open serial communications and wait for port to open:
Serial.begin(9600);
// this check is only needed on the Leonardo:
while (!Serial) {
; // wait for serial port to connect. Needed for Leonardo only
}
Serial.println("Ready....");
// start the Ethernet connection:
if (Ethernet.begin(mac) == 0) {
Serial.println("Failed to configure Ethernet using DHCP");
// no point in carrying on, so do nothing forevermore:
for (;;)
;
}
// print your local IP address:
Serial.print("My IP address: ");
for (byte thisByte = 0; thisByte < 4; thisByte++) {
// print the value of each byte of the IP address:
Serial.print(Ethernet.localIP()[thisByte], DEC);
Serial.print(".");
}
Serial.println();
}
void loop() {
}
You also need to add a couple of lines of code to set pin 10 as output, and set the output to low, otherwise the pin is a floating input pin, which could cause the rfm radio to interfere on the spi bus.Good catch, Kiwi, I failed to see that, with the radio not use, pin 10 won't be declared as an output and SPI won't work, however, to disable the RFM69 CS, pin 10 should be HIGH, not LOW - easy typo.
Thanks KiwiSinceBirth and TomWS! It's all working. The gateway is receiving sensor values and forwarding them via ethernet. The SS is on pin 7.GREAT! Thanks for sticking with it! Glad its working for you!
Good catch, Kiwi, I failed to see that, with the radio not use, pin 10 won't be declared as an output and SPI won't work, however, to disable the RFM69 CS, pin 10 should be HIGH, not LOW - easy typo.Yes when I wrote that (on train) I wasn't quite sure. bborncr, Glad it is working
Tom
I have been having trouble with the Ethernet module not connecting. Sometimes it worked and sometimes it didn't. After several days of frustration I think I found the problem and a solution. According to many users the WIZNET module can have troubles after a cold start. The fix has been to place a 220nF cap between RESET and GND. I just performed a dozen resets without issue. The suggested value for the cap is 100nF but I had the 220nF so I used it instead.Good to know! Thanks for the tip!
if (rxstate == TXRECV && rxfill == 0 /*&& (Byte(0x00) & (RF_RSSI_BIT >> 8)) == 0*/)
3. I choose to change the Ethernet SS Pin, this requires a change to the Ethernet library, I published the change here. https://github.com/kiwisincebirth/Arduino/tree/master/Ethernet
Kiwi
Hi Felix!Yeah I had to discontinue RFM12B since there's really low interest in that module and also much lower performance than the RFM69. Nice project by the way.
I tried buy a Moteino in your store to use in my tests but the RFM12B is out of stock.
If you want know more details about what I'm doing look this topic (http://www.ferduino.com/forum/viewtopic.php?f=20&t=245).
Best regards.
Hello guys!Unfortunately no, the problem is that it isn't a simple change. Felix's RFM12b library doesn't actually use the SPI library, I believe it talks natively to the underlying HW. Thus a complete rewrite of the RFM12b library would be required.
Kiwi, any chance to make this changes to RFM12B library ?
Thanks in advance!
Best regards.
Hi there, in summary, since I lost my more detailed posting.
1. Use arduino 1.6.0, it contains the SpI transaction support to allow the two devices to play nice together.
2. Use the enhanced rfm69 library on my GitHub account, this adds the spi transaction support to the library.
3. I choose to change the Ethernet SS Pin, this requires a change to the Ethernet library, I published the change here. https://github.com/kiwisincebirth/Arduino/tree/master/Ethernet
Lastly, the Ethernet shield is 3.3v compatable, but does require 5v input power. In my setup I have a moteino mounted to a prototype shied which then has an Ethernet shield mounted to it. This means I am free to route any pin I want to pin 10 on the connector, to drive the Ethernet shield
I have been running my gateway without issue for well over a month now, the good thing is that I don't have worry about shutting it down correctly (like rasp pi) there is no OS or sd card to cause an issue.
Kiwi
Hello,Hi Rob, sorry for late reply I am not very active on this forum.
I'm building a RFM69 gateway connected to a W5200 DRDuino Ethernet shield mounted on a Uno R3 board. I've found this thread enormously helpful in working out what to do. However I still have a few questions that I'm hoping someone can help with:
Many thanks for any advice
Rob
Update: While this thread has been dead for many months, my gateway is NOT. Has been running without an issue or any reboot required whatsoever. The good thing is that I can disconnect power without any issue whatsoever, unlike the RPi which it replaced. Very happy indeed.Glad to hear it, Kiwi, and glad to have you back! I've recently added a gateway with Ethernet using your library and, I, too, am pleased at how well it's working. No issues at all!
In summary here is my overall setup.
Remote: Sensor-(Moteino/RFM69) ==>> Gateway: (RFM69/Moteino)-EthernetShield ==>> Server: MQTT -> NodeRed -> Other Applications
Kiwi
Disabling the interrupt during the Ethernet Slave selection solves definitively the problem (my gateway is running for days now, while before it was hanging after few minutes). I suppose that this issue only happens with two asynchronous functions (Ethernet transfer and RFM interrupts).The SPI transaction support is supposed to be doing this (disabling interrupt) automatically. You shouldn't need to do it explicitly in the ethernet code.
If radio reception is synchronised (or chained) with an Ethernet one (or vice versa). The problem should not occur.
Tom,Yes, of course you do. I forgot that the RFM69 library hasn't implemented SPI transactions! I have this implemented in my RFM69_ATC wrapper, which is why I don't have the problem. Kiwi's change to W5100 was to make the SS pin variable, but he also implemented SPI transactions in his version of the RFM69 library to get it to work.
As a matter of fact I have to do it, and I am using a brand new copy of the Arduino IDE 1.6.5-r5.
I also did try the W5100.h Kiwi version with the SPI version of Paul Stoffregen (see https://github.com/PaulStoffregen/SPI), without success.
But I don't want to continue to argue, this work fine for me.No argument, just trying to understand what the problem was and you are exactly right, disabling the RFM69 interrupt during Ethernet SPI access (or any other SPI access for that matter) is necessary.
Tom,Unfortunately, Kiwi's changed RFM69 might be too dated at this point to include directly.
It is becoming very difficult to follow the RFM69 library variants.
I had a look at the RFM library of Kiwi
https://github.com/kiwisincebirth/RFM69/blob/master/RFM69.cpp
which looks to integrate the change request :
https://github.com/LowPowerLab/RFM69/pull/25/files#diff-2d1040d890d26d4ac6877f9269b3e32fR36
This is probably a fix for my issue (didn't try it yet)
However looking at the new releases of the RFM library of Felix, I don't see these changes, however your 'vitalisation' is now included.At the time these changes were made, I think Felix thought they weren't sufficiently tested enough and required a new (at the time) version of Arduino IDE (1.6.0). Also, I was trying to incorporate my ATC changes into the base library. There were also proposals being made to add enhanced security features to RFM69 and the sense was that there wasn't a single combination of these requested changes that everyone wanted.
Any ideas why the change above was not implemented, it would greatly improve the management of all the RFM versions and not to patch standard code.
Hey Robert (or Kiwi),TD, your confusion is justified. There were several iterations of changes to RFM69/W5100 code during the SPI Transaction conversion. Without going into detail (I probably have forgotten most of it anyway), the RFM69 library from LowPowerLab did not implement the SPI transaction code. I ended up putting SPI Transaction support in the RFM69_ATC library as an extension to the base RFM69 library as overrides to the base code.
I was wondering if you could help me understand some of the code in your transaction based RFM69 class.
In select() (lines 461-462), it's saving SPCR and SPSR (SPI status register) and then restoring them in unselect() (lines 489-490) but I thought that the point of the SPI transaction system was that beginTransaction()/endTransaction() takes care of this. Is this in case some other library isn't using transactions or am I confused (which I assume is the answer)?
The biggest thing I don't understand is the interrupt handling in receiveDone() (lines 380-407). The comment by noInterrupts() (line 384) says that the interrupts will get re-enabled in unselect() but the call to interrupts() in unselect() is commented out in the transaction based code. The original line in Kiwi's code has a comment about why SREG is manipulated but I didn't really understand it at all. So I was wondering if you (or anyone) could explain why receiveDone() is saving/restoring SREG which wasn't necessary before and why it calls noInterrupts() but never calls interrupts()? Every other call to interrupts()/noInterrupts() is no longer needed w/ the transaction code so it was a little confusing to me to see noInterrupts() still there on line 384.
Thanks,
TD
//=============================================================================
// select() - replaces base class with one supporting spiTransactions
//=============================================================================
// select the transceiver
void RFM69_ATC::select() {
// save current SPI settings
_SPCR = SPCR;
_SPSR = SPSR;
_SREG = SREG;
#ifdef SPI_HAS_TRANSACTION
SPI.beginTransaction(SPISettings(4000000, MSBFIRST, SPI_MODE0));
#else
noInterrupts();
// set RFM69 SPI settings
SPI.setDataMode(SPI_MODE0);
SPI.setBitOrder(MSBFIRST);
SPI.setClockDivider(SPI_CLOCK_DIV4); // decided to slow down from DIV2 after SPI stalling in some instances, especially visible on mega1284p when RFM69 and FLASH chip both present
#endif
digitalWrite(_slaveSelectPin, LOW);
}
//=============================================================================
// unselect() - replaces base class with one supporting spiTransactions
//=============================================================================
// UNselect the transceiver chip
void RFM69_ATC::unselect() {
digitalWrite(_slaveSelectPin, HIGH);
#ifdef SPI_HAS_TRANSACTION
SPI.endTransaction();
#else
SREG = _SREG;
#endif
// restore SPI settings to what they were before talking to RFM69
SPCR = _SPCR;
SPSR = _SPSR;
}
TomWS,Curious... My own version of the ATC library has the code I posted a little while ago. I'll have to take a look at the various versions, but I can't do it today. Maybe this weekend.
Thanks for the reply. But - the ATC library at LowPowerLab and in your own github repository doesn't contain any transaction code. Can you update your github repository with the latest version?
And - in your example, could you comment on why the transaction code needs to save and restore SPCR, SPSR, and SREG? My understanding (probably wrong) is that if all the SPI devices are using transactions, saving and restoring these registers shouldn't be necessary.
Thanks,
TD
Hi Felix,Hi Giacomo,
did you managed to test the SPI support?
3. Most Important add SPI.usingInterrupt(_interruptNum) in the bool RFM69::initialize(uint8_t freqBand, uint8_t nodeID, uint8_t networkID)THAT is sort of key to the whole thing working, isn't it!!! ;)
……..
while (((readReg(REG_IRQFLAGS1) & RF_IRQFLAGS1_MODEREADY) == 0x00) && millis()-start < timeout); // wait for ModeReady
if (millis()-start >= timeout)
return false;
//!!!! ROB
#ifdef SPI_HAS_TRANSACTION
SPI.usingInterrupt(_interruptNum);
#endif
//!!!!!
attachInterrupt(_interruptNum, RFM69::isr0, RISING);
selfPointer = this;
_address = nodeID;
return true;
}