main.cpp is very big

[rlkeck]

First let me say I’m impressed by OpenSprinkler. It works well and is quite versatile. Obviously a lot of effort has gone into it. But…

The firmware file main.cpp is nearly 1700 lines long. It would be nice if were split into functional units, e.g. logging.cpp, scheduler.cpp, web.cpp, and so on so that do_loop() looked more like simply a list of tasks to be processed:

do_loop(){
  doNetwork();
  doSchedule();
  doLog();
  ...
}

As it stands, it is really easy for anyone not really familiar with the code to get lost in main.cpp while trying to make modifications to OpenSprinkler. In addition splitting main.cpp into functional sub-units would make it easier to port OpenSprinkler to a multi-threaded OS like FreeRTOS.

[Ray]

It’s true that main.cpp has become quite large and we haven’t done much to optimize the code design. The main challenge is that it has many #ifdef sections to address different hardwares (for example the same function may have several implementations one for each target hardware). On the other hand, I am also worried that splitting it into different files would make it difficult for finding a specific function, and since many functions are inter-related, splitting them into different files also means they have to include the function declarations of each other, which will further increase the code size. Will see what we can do to make the code more clear in the next version. Thanks for the suggestion.

[testuser]

While there are a lot of #ifdef’s, I don’t think that is what will make splitting things hard. Finding functions is not much of a problem with modern tools (Visual Studio, VScode, Eclipse and others) since they all can find definitions and declarations in other files. Including headers should not increase the size of the binary image unless you are doing something very strange in the headers.

I have found that refactoring code like this often results in the end in much more structured code with fewer interdependencies although there may be a certain amount of pain getting there. I guess I’d say that the OpenSprinkler code might benefit from more extensive use of classes. For example one way to deal with hardware differences is to define an abstract base and specialize the hardware, e.g.

class I2C
{
public:
  virtual int read(...) = 0;
  virtual byte write(...) = 0;
  ...
};
 
class RPiI2C : public I2C
{
public:
  int read(...); //implementation in cpp file
  byte write(...); //implementation in cpp file
  ...
};

Then where you want to use it:

#ifdef RPI
RPiI2C i2c;
#else
ArduinoI2C i2c;
...
#endif
 
i2c.read(...);

I admit I am partial to classes since my preferred language is C#, where all code must be in a class. Anyhow, anything you can do to shrink main will be appreciated.

I have forked your project in order to add a moisture sensor to it and have considered doing some refactoring myself. The problem with that is it will make it difficult for me to merge any changes you make back into it.

[Ray]

“While there are a lot of #ifdef’s, I don’t think that is what will make splitting things hard” — that’s not what I meant — what I meant was that those ifdefs are what’s making the file large.

What you mentioned about creating abstract classes is what we did for classes like GPIO.h/.cpp. On the other hand, main.cpp is based and evolved from legacy code (OpenSprinkler 1.0) for which I had to do a lot of tricks to maximally save flash memory space and RAM. That’s why the code there is largely standalone functions with no classes. Later as we moved on to 2-nd generation and 3-rd generation, the microcontrollers have a lot more flash memory and RAM, so some of the new classes created can use more OOP features.

[Author]

Posts