sandeepmistry / arduino-LoRa

An Arduino library for sending and receiving data using LoRa radios.
MIT License
1.67k stars 634 forks source link

Compile error with getSignalBandwidth: control reaches end of non-void function #201

Closed mfarabee closed 3 years ago

mfarabee commented 6 years ago

I was getting an error in the function getSignalBandwidth when compiling: Arduino: 1.8.1 (Windows 10), Board: "TTGO LoRa32-OLED V1, 80MHz, 115200, None" C:{...path...}\Arduino\libraries\LoRa\src\LoRa.cpp: In member function 'long int LoRaClass::getSignalBandwidth()':

   C:\{...path...}\Arduino\libraries\LoRa\src\LoRa.cpp:437:1: error: control reaches end of non-void 
    function [-Werror=return-type]
   }
  ^
  cc1plus.exe: some warnings being treated as errors

I tracked this down to the following: 1) I have compiler warnings set to "More" in the Arduino IDE (File -> Preferences -> Compiler Warnings: “More”) 2) The way the function is written, it could have a case where it drops out without having a return value. Obviously this won't happen because the data is known, but the compiler does not know this.

To fix this either a dummy return statement needs to be placed after the switch statement or a default case needs to be implemented. I just added "return(0L)" after the switch statement.

Thanks! Great work on this library!

torntrousers commented 5 years ago

Thanks! Care to submit a pull request? Or if not I can do it as I added that getSignalBandwidth function.

mfarabee commented 5 years ago

If you would not mind doing it, that would be great. I have never done that before. I also am not sure the best way to implement it. Just in case the switch statement does drop out, what should be the default value? 0L? 1L? bw? 7.8E3? 500E9? Does not matter?

One fear I have is if someone makes a change to the setSignalBandwith code such as adds another case without updating the getSignalBandwith routine.

I do see that the getSignalBandwith is used is two other routines (packetFrequencyError, getLdoFlag). We would want to return an acceptable value to theses functions. I think "0L" was a bad implementation for my local fix because now I see that getLdoFlag could create a divide by zero. I have changed this in my local version to "1L".

Returning bw could be good for error checking. You would be able to see what caused the error condition. return((long}bw); // To eliminate compiler warnings. This statement should never be reached.

I think the best may be, "1L" return(1L); // To eliminate compiler warnings. This statement should never be reached.

I will leave the implementation to you as you know much more about this than I do. Just hope my observations are helpful. Thanks!

GusakIurii commented 5 years ago

I have a similar problem. To remove the error, I did this: long LoRaClass::getSignalBandwidth() { byte bw = (readRegister(REG_MODEM_CONFIG_1) >> 4); switch (bw) { case 0: return 7.8E3; case 1: return 10.4E3; case 2: return 15.6E3; case 3: return 20.8E3; case 4: return 31.25E3; case 5: return 41.7E3; case 6: return 62.5E3; case 7: return 125E3; case 8: return 250E3; case 9: return 500E3; default: return 125E3; } }

torntrousers commented 5 years ago

This was fixed in master sometime ago, there just hasn't been a release for a while to include it.

GusakIurii commented 5 years ago

Thank you for your work. I hope I made the changes right? I just left here the current solution to this problem .. For example, I did not immediately understand what the error was ..

AloisAtGitHub commented 5 years ago

Version 0.5.0 works well on an Arduino Pro (Atmega328), using an ESP32 i also get this Error. (LoRa.cpp: In member function 'long int LoRaClass::getSignalBandwidth()':) On the ESP32 i have to use the version 0.3.0

torntrousers commented 5 years ago

@morganrallen any chance of a new release to pick up this and all the other fixes since 0.5.0?