search

timmbogner / Farm-Data-Relay-System

A system that uses ESP-NOW, LoRa, and other protocols to transport sensor data in remote areas without relying on WiFi.
MIT License
485 stars 108 forks source link

Bug: Fixing sensor and gateway configuration #71

Closed Gulpman closed 1 year ago

Gulpman commented 1 year ago

Although there is a fdrs_globals.h containing a global configuration, this is not used stringent within the code.

The configuration system should work as following:

  1. If a node setting is configured locally, use that value, otherwise use the value from the global configuration file.
  2. If neither the local or global setting is configured, throw an error.

Within the sensor and gateway logic, at first the config must be checked and the FDRS_ variable set accordingly. After this assignment has happened, only FDRS_ variables must be used.

Gulpman commented 1 year ago

In commit https://github.com/timmbogner/Farm-Data-Relay-System/pull/72/commits/5027623cb8a911513f329f0d8d8dd9bcde2e5eb1 the configuration systems now is implemented for gateways as well. I didn't touch Universal_Gateway_beta though. So stick with the sensors and gateways which are not class-based. :)

I am still struggling with how to do a pull-request: First opening an issue and then doing the pull request in this case led to #71 and #72 Sorry for the confusion. What I actually wanted was to open this thread, and then add the pull-request to it.

I'm also not fully happy with having a debugConfig function in both the sensor and gateway files. But the function itself helps in checking the config a lot. Maybe it would make sense to have a file holding shared code, e.g. fdrs_common.h or something like that?

Gulpman commented 1 year ago

I just see that it is not possible to use WiFi (for MQTT) and ESP-Now at the same time in the gateway code. This isn't checked for at the momemt, but detecting that both have been enabled and warning the user should be implemented as well. Todo for tomorrow. :)

timmbogner commented 1 year ago

Sorry if this comes a little late, but I just realized that we can solve it all by only defining globals in globals.h, and the rest in config.h .

Having the option to define them in both places is less important now that installation is mandatory.

So... anything defined in globals should be removed from config. That makes things way easier right?

Gulpman commented 1 year ago

Sorry if this comes a little late, but I just realized that we can solve it all by only defining globals in globals.h, and the rest in config.h .

Maybe I do not understand correctly, but defining globals in fdrs_globals.h and individual settings for a specific sensor in fdrs_sensor_config.h and a specific gateway in fdrs_gateway_config.h is what I have done with this commit.

Having the option to define them in both places is less important now that installation is mandatory.

I think there are enough use cases for someone to define specific settings locally. E.g. I like to have individual MQTT credentials for all nodes just in case one is compromised. There may be the need for a different SF for some nodes/gateway combos because there is interference ... and so on.

So... anything defined in globals should be removed from config. That makes things way easier right?

This is possible - and it would make things easier, for sure but less flexible. Me personally I can live with it. :)

Apart from which way we go: what is definitively missing is an overview on the current config of a node at start-up. I found myself so many times trying to find out why nodes couldn't communicate - an overview of the node's configuration would have helped a lot. I'm actually working on it. Output could be something like that (showing what I already have implemented):

====================================================
FULL CONFIG OVERVIEW
====================================================
----------------------------------------------------
NODE Ids:
Node Type: Sensor
READING_ID: 2
GTWY_MAC: 0x04
----------------------------------------------------
OVERVIEW ON PROTOCOLS
ESPNOW: enabled
MQTT  : disabled
LORA  : enabled
----------------------------------------------------
SENSOR LORA CONFIG:
LoRa Band used from GLOBAL_LORA_BAND: 866000000.00
LoRa SF used from GLOBAL_LORA_SF    : 7
----------------------------------------------------
...

Above is sensor and gateway info mixed, I know - it is just as a showcase for what would be displayed there. :)

I moved this to an external file called fdrs_checkConfig.h which only is included if the corresponding macro is defined. This way I can remove the debugConfig() function from fdrs_functions.h and fdrs_sensor keeping them lean.

Talking about simplicity, I do have the following proposals:

timmbogner commented 1 year ago

Ope, guess I was thinking this got wrapped up in the PR. All of these changes sound reasonable! fdrs_gateway.h is preferred. At some point I want to talk about your idea to replace floats while in transit too!

timmbogner commented 1 year ago

The external checkConfig.h is a good call. I've been considering adding a separate header file for all non-crucial functions like this. That way we can add lots of extra tools but still easily disable some or all non-critical features for troubleshooting etc. We could possibly even separate a config file for those features. You're keen on organization, so I'd love to hear your thoughts.

Gulpman commented 1 year ago

There is still some minor cleanup to be done in the config section. I also need to add support for SD cards and the new ACK settings. As I have to finish a project till Wednesday, I can start doing the changes after that.

Gulpman commented 1 year ago

I couldn't test the LoRa sensor node as it does not not compile atm:

In file included from D:\_sync\Sascha\OneDrive\Documents\Arduino\libraries\Farm-Data-Relay-System\examples\LoRa_Sensor\LoRa_Sensor.ino:10:0:
D:\_sync\Sascha\OneDrive\Documents\Arduino\libraries\Farm-Data-Relay-System\src/fdrs_sensor.h: In function 'crcResult getLoRa()':
D:\_sync\Sascha\OneDrive\Documents\Arduino\libraries\Farm-Data-Relay-System\src/fdrs_sensor.h:235:51: error: 'transmitLoRa' was not declared in this scope
             transmitLoRa(&sourceMAC, &pingReply, 1);
                                                   ^
D:\_sync\Sascha\OneDrive\Documents\Arduino\libraries\Farm-Data-Relay-System\src/fdrs_sensor.h:257:51: error: 'transmitLoRa' was not declared in this scope
             transmitLoRa(&sourceMAC, &pingReply, 1);
                                                   ^
exit status 1
Fehler beim Kompilieren für das Board ESP32 Dev Module.
timmbogner commented 1 year ago

The generic "fdrs_node.h" (soon fdrs_client.h?) file is about to replace fdrs_sensor.h. The LoRa sensor compiles in the two-way-comms branch if you change the include to "fdrs_node.h".

What I'm hoping to accomplish today is to make this and other changes in all of the the examples so that two way comms can merge into dev.

I am also considering adding letters to denote sensor examples "a_ESPNOW_Sensor" "b_LoRa_Sensor". Would you prefer this?

Good to see you!