Esp8266_deauther: Refactoring source code regarding header file / declaration, definition and one definition rule

[xdobry]

The project is probably the biggest esp8266 one. Thanks for making it public

I analyze the sources for a while and found that some things do not follow
c/c++ conventions.
Last years I do not use cpp often but some years ago I did a lot gcc.

Some words german

Offensichtlich funktioniert so wie es jetzt ist auch aber bei Gelegenheit könnte man es
bereinigen. Es könnte aber sein, dass bei bei Microcontroller code ganz anders vorgeht.

There may be a problem about what to define in header files and what in compilation unit (*.cpp).

I have reworked for me all headers. The compilation is faster and resulted output is smaller.
Also changes does not force to recompile all compilation unit.

There some best practices for header files in cpp

• do not define function in header files (except inline and templates). You should only declare functions
• do not repeat yourself (there are planty same extern .... function declarations)
• the header function includes should be minimal. Only includes that are needed for declaration not for definition. This will make compilation of units faster. And dependencies clearer
• you may use static function. Static means private/local for one module

It is interesting to know how cpp compilation works.
There are 2 phases. See compiler output.
The compilation units are created (the .o files) and then all is linked together.
So if you have many includes in header files. You probably will have much more code
to compile for one unit (.o file). It is slower.
In the case of the project including one header follow to have all header for all files.
It is not necessary.

You may use
xtensa-lx106-elf-nm sketchDisplayUI.cpp.o
and
xtensa-lx106-elf-nm esp8266_deauther.ino.elf
to dump all symbols in compile unit or linked binary

Example (Now)

Display.h

include "Arduino.h"

include

include

extern "C" {
#include "user_interface.h"
}

include "language.h"

include "A_config.h"

include "Settings.h"

include "Names.h"

include "SSIDs.h"

include "Scan.h"

include "Attack.h"

DisplayUI.cpp

include "Display.h"

Better is to keep Display.h minimal and move the includes needed for definition to DisplayUI.cpp
After rework

DisplayUI.h

include "Arduino.h"

include

include "SimpleList.h"

include "A_config.h"

DisplayUI.cpp

include "DisplayUI.h"

include

include

extern "C" {
#include "user_interface.h"
}

include "language.h"

include "A_config.h"

include "Settings.h"

include "Names.h"

include "SSIDs.h"

include "Scan.h"

include "Attack.h"

include "Accesspoints.h"

include "Stations.h"

include "wifi.h"

This way also the dependecies are clear.
What is needed for declaration and what for definition.

---

you could define as many arduino libs the object already in Display.h
or declare its as extern. So you do not need to write many times

extern DisplayUI displayUI

---

function.h should have function.cpp und function.h
Do not define function in function.h.

The definition would be duplicated

Same for wifi.h
It could be following

ifndef WifiManager_h

define WifiManager_h

include "Arduino.h"

define WIFI_MODE_OFF 0

define WIFI_MODE_AP 1

define WIFI_MODE_STATION 2

void stopAP();
void wifiUpdate();
void startAP(String path, String ssid, String password, uint8_t ch, bool hidden, bool captivePortal);
void printWifiStatus();
void startAP();
void startAP(String path);
void loadWifiConfigDefaults();
void resumeAP();

extern uint8_t wifiMode;

endif

All over move to new unit wifi.cpp

language.h is also not standard way
in language.h the should be only extern ......

https://stackoverflow.com/questions/5499504/shared-c-constants-in-a-header

---

Perhaps some new global.h file with all externals. I am not sure what is better.

external Scan scan;

See also
https://en.wikipedia.org/wiki/One_Definition_Rule

[spacehuhn]

Hey, Iappreciate your advice!
If you didn't already, please have a look at my issue here regarding exactly this topic: https://github.com/spacehuhn/esp8266_deauther/issues/846
It may got washed away by the other issues and I haven't had time to go through everything yet. I notice now that I should also update the spaghetti-code-issue!

I see the problem less in the includes and file structure (although you're absolutley right about that and I'm working on that too!) but more on the architecture of certain functionalities themselves. Because if those things were programmed in a better matter, there would be no need to even create those clusterfucks of includes and externs. But a lot parts of the code are just not doable in a perfect clean way unless you make a lot of changes to a lot of different parts in the code. Which is why I'm taking time now to make independent libraries for the most important and more complex features.

And yeah especially language.h and function.h are a mess that I do not intent to recreate with the next rewrite I'm working on.

[xdobry]

My impression was also that it looks like "spaghetti-code-issue" but I did not want to say it ;-).
I also thought about it. Here some of the considerations about redesign. Perhaps it can be useful.

Currently many modules references each other. There are circular references.

SerialInterface references Scan, DisplayUI, Wifi
Wifi references Scan
Scan references Wifi

The another problem is that program "state" in spread among many global variables and states.
To solve the spaghetti-code-issue one need to decouple this modules.
The standard solution will be to create new unit like State or EventManager

Wifi references State
Scan references State

To get inspiration how to do it good place is game programming, because it is quite similar to microcontroller arduino programming.

• There is loop (game loop)
• There is need for speed and optimization and limited resources
  Look at
  http://gameprogrammingpatterns.com/contents.html
  And especially at
  http://gameprogrammingpatterns.com/state.html
  http://gameprogrammingpatterns.com/event-queue.html

In the project there are also 3 kind of user interface (Web, DisplayUI, SerialInterface)
Quite many.
The old way to handle it is "MVC" Model View Controller pattern.
But unfortunately there are many different implementation and kinds of it.
But most of them use some kind of event dispatching or public/subscribe pattern.

By the way the best part is for me is DisplayUI menu programming.
It is quite clever and impressive and even works for dynamic structures.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.