First of all, thank you for making you great library open source.
In the last months I studied the code quite intensively and found two minor points of potential improvements.
1) Concerns RFM69_ATC: If ATC is activated and the recipient does not return ACKs, the sender dials up the power level til it reaches 31. It would be nice, if the maximal powerlevel could be limited to a value defined by the user. This could be achieved through a second optional parameter to the setPowerLevel function specifying the maximum value to which the power is dialed up when the recipient is not online.
2) Concerns the sendACK and canSend methods:
RFM69::canSend()
{ // CSMA_LIMIT = - 90
if (_mode == RF69_MODE_RX && PAYLOADLEN == 0 && readRSSI() < CSMA_LIMIT) // if signal stronger than -100dBm is detected assume channel activity
{
setMode(RF69_MODE_STANDBY);
return true;
}
return false;
}
// should be called immediately after reception in case sender wants ACK
void RFM69::sendACK(const void* buffer, uint8_t bufferSize) {
ACK_REQUESTED = 0; // TWS added to make sure we don't end up in a timing race and infinite loop sending Acks
uint8_t sender = SENDERID;
int16_t _RSSI = RSSI; // save payload received RSSI value
writeReg(REG_PACKETCONFIG2, (readReg(REG_PACKETCONFIG2) & 0xFB) | RF_PACKET2_RXRESTART); // avoid RX deadlocks
uint32_t now = millis();
while (!canSend() && millis() - now < RF69_CSMA_LIMIT_MS) receiveDone(); // RF69_CSMA_LIMIT_MS = 1000 ms
SENDERID = sender; // TWS: Restore SenderID after it gets wiped out by receiveDone()
sendFrame(sender, buffer, bufferSize, false, true);
RSSI = _RSSI; // restore payload RSSI
}
It seems that there are some RFM69 modules which do not return the correct actual RSSI Level in the readRSSI() function. In your library the threshold CSMA_LIMIT is set to -90 but those modules return values of about -70 or -80 that the canSend method always returns false until the timeout RF69_CSMA_LIMIT_MS (set to 1000 ms) has expired. This timeout seems to be to long since in the sendWithRetry method the longest possible wait time for an ACC is 256 ms. So the transmitter will evt. fire the full number of retries as it doesn't get the ACK in the time window of 256 ms. So it would evt. be better to set the RF69_CSMA_LIMIT_MS to e.g. 200 ms.
It should be considered to let the CSMA_LIMIT flow in a range from lets say -60 to -90 through a function like this in the send and sendACK method.
//replace in sendACK: while (!canSend() && millis() - now < RF69_CSMA_LIMIT_MS) receiveDone();
//with something like this:
uint32_t StartTime = millis();
while (true)
{
if (canSend())
{
_adjusted_CSMA_LIMIT = _adjusted_CSMA_LIMIT - 1;
_adjusted_CSMA_LIMIT = _adjusted_CSMA_LIMIT < -90 ? -90 : _adjusted_CSMA_LIMIT;
break;
}
if (millis() - StartTime > RF69_CSMA_LIMIT_MS)
{
// Level is stepped up from -90 in steps of 2, but not higher than -60
_adjusted_CSMA_LIMIT = _adjusted_CSMA_LIMIT + 2;
_adjusted_CSMA_LIMIT = _adjusted_CSMA_LIMIT > -60 ? -60 : _adjusted_CSMA_LIMIT;
break;
}
receiveDone();
}
Kind regards
RoSchmi
