Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change key-names in JSON to be compliant and consistent #1860

Closed
proddy opened this issue Jul 11, 2024 · 8 comments
Closed

Change key-names in JSON to be compliant and consistent #1860

proddy opened this issue Jul 11, 2024 · 8 comments
Labels
technical Technical enhancement, or tech-debt issue
Milestone

Comments

@proddy
Copy link
Contributor

proddy commented Jul 11, 2024

The key names are inconsistent in the downloaded emsesp_settings.json file which is the same as the API call http://ems-esp.local/api/system.

I'd like to rename the keys to either camel-case or snake-case, following the standards that the JSON org and Google recommend: https://restfulapi.net/valid-json-key-names/ https://google.github.io/styleguide/jsoncstyleguide.xml?showone=Key_Names_in_JSON_Maps#Key_Names_in_JSON_Maps

This would mean names like "publish time solar" would become "publishTimeSolar" using camel-case. And "Network Info" changes to just "network".

Only the System Info would be impacted. The devices and other entities are in lowercase without spaces and can stay like that.

@tp1de @MichaelDvP

@proddy proddy added the technical Technical enhancement, or tech-debt issue label Jul 11, 2024
@proddy proddy added this to the v3.7.0 milestone Jul 11, 2024
@MichaelDvP
Copy link
Contributor

Yes, i like that.
You know that i have difficulties with these entities and blanks when developing the scheduler-conditions (is solved). It's much easier if we have a consistent naming scheme without blanks and special characters like in api/system/System Info/Uptime (seconds).

  • For the custom names for custom/temperaturesensors/analogsensors/scheduler etc. we should use the same regex pattern, except that for custom/analog the names required, scheduler is optional and temperaturesensors fall back to ID if name is empty.
  • We should have same max. length for all names, mayby <20.
  • We can allow CamelCase, but for unique name we should compare lower case and execute commands case independent.

@tp1de
Copy link
Contributor

tp1de commented Jul 12, 2024

Please consider to align mqtt (ha) and api names.
I understand that you prefer CamelCase. Is this right?

@proddy
Copy link
Contributor Author

proddy commented Jul 12, 2024

Please consider to align mqtt (ha) and api names. I understand that you prefer CamelCase. Is this right?

yes camelCase.

in the API, the URL won't be case sensitive. In HA I need to check if the sensor names are case sensitive. right now, it's all lower-case which is fine. The recommendation from https://github.com/Trikos/Home-Assistant-Naming-Convention is to use underscores, which I don't like.

@proddy
Copy link
Contributor Author

proddy commented Jul 13, 2024

So I propose, for starters, to change to use camelCase and no spaces or special characters in:

  • the API endpoint /system/info
  • the MQTT topic heartbeat. This will also change the MQTT device ems-esp in HA, so old dashboards need refreshing. Code is in Mqtt:ha_status()
  • the MQTT topic info
  • change Web code as Michael mentioned to keep all RegEx consistent (max 20 length), comparing case-insensitive except for the special conditions with temperaturesensor, empty scheduler name etc...

@tp1de
Copy link
Contributor

tp1de commented Jul 13, 2024

@proddy please allow the following remarks / questions:

Whenever names are changed for mqtt (HA) the old entities became inactive and the old statistic records are kept. This is not nice and needs manual intervention next to adjusting dashboards.

Is there any proposed way how to do this with minimum effort? Or can this be done without manual intervention?
While doing the changes in steps. These exercise must be done more than once. I do not recommend.

Within ioBroker I changed the code to recreate the object structure for /system/info on every instance restart.

@proddy
Copy link
Contributor Author

proddy commented Jul 13, 2024

I agree we should avoid making any major changes to the HA historical data. With my plan above, only 3 sensor IDs will change in HA: bus_status, uptime_sec, ntp_status

But we can decide not change that, and it would mean nothing changes in HA.

That leaves

  1. change the MQTT topics payload for heartbeat and info. This is purely cosmetic.
  2. continue with the initial plan to change the /api/ endpoints, starting with system/info so it can be used in the new Scheduler conditions logic and the new Message command. For example today {"System Info"."uptime (seconds)"} is nsaty.

@proddy
Copy link
Contributor Author

proddy commented Jul 25, 2024

I changed heartbeat and info, but it also impacted HA's entities, so I'll leave those two and only change system/info.

proddy added a commit to proddy/EMS-ESP32 that referenced this issue Jul 25, 2024
@proddy
Copy link
Contributor Author

proddy commented Jul 25, 2024

@tp1de note system/info is changed so you'll need to adjust your ioBroker code accordingly

@proddy proddy closed this as completed Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
technical Technical enhancement, or tech-debt issue
Projects
None yet
Development

No branches or pull requests

3 participants