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
507 stars 114 forks source link

Proposal: Move data types to a separate file within FDRS_install #35

Closed Gulpman closed 2 years ago

Gulpman commented 2 years ago

I was wondering if it wouldn't be easier for maintainance and cleaner code if the data types for sensors would go to a separate file within the FDRS_Install folder, e.g.:

FDRS_Install\FDRS_datatypes.h

#define STATUS_T    0  // Status 
#define TEMP_T      1  // Temperature 
#define TEMP2_T     2  // Temperature #2
#define HUMIDITY_T  3  // Relative Humidity 
#define PRESSURE_T  4  // Atmospheric Pressure 
#define LIGHT_T     5  // Light (lux) 
[...]

This way it could be easily included within the sensor sketches by using #include <FDRS_datatypes.h>

This would shorten the fdrs_sensor.h's for each sensor a little bit. But the great advantage would be that if the data types are extended it does not have to be edited in every single sensor sketch to match the newest version. Only updating the file would be sufficient.

Or am I overseeing something?

thefeiter commented 2 years ago

I guess this would also be a good Idea for the fdrs_functions and fdrs_sensor files. Can one make a library out of this repo which one could install in the libraries folder?

thefeiter commented 2 years ago

I would suggest moving all the files containing the same code to a central directory which is to be placed in the libraries folder.

timmbogner commented 2 years ago

The types as globals is also something that Andreas pushed for as well. My reluctance was because I thought those should only be changed project-wide. That is, I wanted people to contact me to add new types, that way we didn't end up with a bunch of conflicting new ones. The other issue that I always stuck to was that I wanted the system to stand alone. Like, you can run it out of its sketch folder with no installation. I was happy to have someone see the repo, let alone move files around just to try it out.

Soon, the library approach will make this a non-issue, as things will be installed as the norm. For now, I think I could have it say:

   #ifdef USE_GLOBALS
   #include <library.h>
   #else
   #include "library.h" 
   #endif

This would choose the header in the project folder if globals is not available. Seem good?

timmbogner commented 2 years ago

I forgot to wrap up my initial thought, which was that the "FDRS_Install\FDRS_datatypes.h" idea seems like a good one.

Devilbinder commented 2 years ago

In favor of moving any duplicate code to the root of the project. Was my original idea with #17 and #20. It will make code maintenance much easier.

However keep the original examples as is due to the video that Andreas made. Would force any new code to follow the library method. The other option is to create a branch at 3818d88dd5fdbed1f2521bdb624bf6ee088c29f4 and direct people to that branch if they want the version from the video.

The other issue that I always stuck to was that I wanted the system to stand alone. Like, you can run it out of its sketch folder with no installation.

You already have 4-5 dependency's that require installation. ArduinoJson LoRa pubsubclient ESP32 or/and ESP8266 boards

P.S. This topic will keep popping up.

timmbogner commented 2 years ago

Okedoke that all sounds reasonable. Here is my plan:

Gulpman commented 2 years ago

Okedoke that all sounds reasonable. Here is my plan: [...]

  • We'll do the thing with the data types.

Done :)

* I always appreciate these kinds of suggestions! I'm figuring this out as I go. I just did a deep-dive into best practices for Arduino libraries and APIs and it's been very interesting. I didn't even realize i was designing an API, let alone such a crappy one! 😅

I had a look at how to deploy code as a library yesterday and found the following article very helpful: https://roboticsbackend.com/arduino-create-library/