search

s00500 / ESPUI

A simple web user interface library for ESP32 and ESP8266
https://valencia.lbsfilm.at/midterm-presentation/
Other
932 stars 174 forks source link

Moved ControlType and ControlColor objects into class specific namesp… #308

Open MartinMueller2003 opened 4 months ago

MartinMueller2003 commented 4 months ago

…ace. This resolves a conflict with some other libraries that also use global definitions.

MartinMueller2003 commented 4 months ago

The open questions are:

Is this OK Should I take it further, Things like the control array are public and they should be made private.

s00500 commented 4 months ago

Hm... so this creates a bit of a breaking changes, but I can see the defines are still there ?

I like the idea of addressing the incompatibilities... and I think it is fair enough to go about them like so. I dont think we have any other libraries that have ESPUI as an upstream dependency (not like ArduinoJSON) so I am also less concerned...

As for taking it further, any reason why the control array is public ? I assume nobody uses it in user code right ? Else I would guess it makes sense to go further, any other issues you have in mind here ?

MartinMueller2003 commented 4 months ago

I have no idea why it is public. We have accessors to get a control by ID so I do not see a reason for it. The other major breaking change would be to modify the control structure so that its fields are all private. Then force people to use an accessor to modify the values. That will be a significant change to the applications that are taking shortcuts.

As for the #defines, the names are pretty unique and the value is now class specific so the chances for a collision are smaller. If you really want to segregate the code, we could put the ESPUI code into its own name space and then access it with a "using" statement.

MartinMueller2003 commented 4 months ago

Just added upgrade instructions to the readme and bumped the version to 2.3.1

s00500 commented 4 months ago

The ESPUI Verison ? I would rather do that when releasing, not on the merge, but I agree that this should be the next version number.

Shall I release current master as 2.2.4 now to include your upgrades for ArduinoJSON or should we wait till this pr is merged ? (I think a release now makes sense)

MartinMueller2003 commented 4 months ago

I would release it as it is. and then we add the breaking change.

s00500 commented 4 months ago

did release and updated PIO :-)

MartinMueller2003 commented 4 months ago

Update the control management process one level up in OOO. ID and type are now protected by accessors and are now read only.

The management of the list of controls has been gathered into a single file called ESPUIcontrolMgr. This is a singleton factory class responsible for creating, deleting and manipulating controls.

This is about as far as I would want to take things at this time. There are more OOO design patterns that can be applied, but they would be onerous on the existing user base.