Aircoookie / WLED

Control WS2812B and many more types of digital RGB LEDs with an ESP8266 or ESP32 over WiFi!
https://kno.wled.ge
MIT License
14.31k stars 3.05k forks source link

Expose WLED_RELEASE_NAME via API and Device info page #3497

Closed jamesmyatt closed 2 months ago

jamesmyatt commented 8 months ago

I'm looking at adding updating the FW for ESP32-C3, -S2 and -S3 via Home Assistant: https://github.com/frenck/python-wled/issues/1140

It seems like the easiest way to do this would be to get the value of the WLED_RELEASE_NAME from the API. Could this be added? e.g. as an optional field in /json/info?

edit I'd also like to see this on the device info page and the update page in the Web UI.

dosipod commented 8 months ago

Cant you do that with "arch" and "ver" ? image

jamesmyatt commented 8 months ago
  1. There are 3 different releases for arch=ESP8266 (ESP01, ESP02, ESP8266), 3 for arch=ESP32 (Ethernet, audioreactive and standard) and you need to add "_8MB" for arch=ESP32-S3 (https://github.com/Aircoookie/WLED/releases/)
  2. What new releases will be added in the future? Will there be other flash sizes supported? Other chips? Other builds with different features (c.f. audioreactive)?
  3. What about improving support for other forks such as the semi-official MM and SR ones (https://mm.kno.wled.ge/) that have entirely different release names? (https://github.com/MoonModules/WLED/releases/)

All of this work can be avoided by just providing WLED_RELEASE_NAME via the API.

blazoncek commented 8 months ago

1) different releases are primarily needed for serial flashing (as it establishes correct partition sizes). once partitions are set up, you can update OTA any release you choose (except if file size exceeds available flash partition size) 2) it will be impossible to list (and provide) all possible combinations of usermods or features 3) it is up to the respective fork to establish how they implement anything. they may or may not include something from upstream (as they do ATM)

jamesmyatt commented 8 months ago
  1. I think you're saying that you can OTA any ESP8266 FW on any ESP8266 board, because the only difference is flash size and the features are always the same. But I don't think that is always necessarily true, and future versions may have different features in different ESP8266 releases. Also there are 3 different official feature builds for ESP32 that you need to match. I don't think its too much to ask to provide enough information via the API to be able to easily identify exactly which of the official releases from https://github.com/Aircoookie/WLED/releases/ you have loaded at any point in time, especially if it's easy to do. If it's not easy, then that's another matter.
  2. Sure, but this is only really concerned with the official releases from https://github.com/Aircoookie/WLED/releases/ and if new ones are added to that standard list, it would be helpful for tools that work with WLED to be able to automatically support them rather than code each specifically. I think this approach scales automatically to new official releases. Unless WLED_RELEASE_NAME isn't the right parameter for the released asset names.
  3. Sure, but it's good for the Aircoookie version to be setting a standard API that contains placeholders that can work for everyone, including the forks. Which (I assume) is why there's already "brand" and "product" fields in the info API. If forks don't use those fields in a way that's consistent with the official Aircoookie releases, then they can't be expected to work correctly with rest of the ecosystem.

Regardless of whether this would be helpful, this is also important debugging information just like the build number that is shown on the update and device info pages in the web UI. In fact, I'd like to see the release name on those pages in the UI as well. And I've edited the OP to add this.

blazoncek commented 8 months ago

1) you are missing the point. re-read my post. besides, there can be as many different "releases" as there are compile-time options or (more likely) build environments. None of that is happening officially (in the near future). The three release versions for ESP8266 are there just because of 3 different flash sizes (and the 1M version is stripped of OTA capabilities since flash size will not allow it). 2) the only two "official" "optional" releases are Ethernet and Audioreactive (which is technically a usermod but was requested numerous times), all other "releases" feature the same functionality and build options (if there weren't that many GPIOs reserved for Ethernet I would argue to include Ethernet option in every ESP32 build) 3) I may be thinking in a wrong direction, but WLED is not a commercial software it is a DIY project still. So are some forks.

As far as I can tell there are official releases with:

There are plenty more build environments some of which include variants with PSRAM support, disabled core functionalities (IR, OTA), enabled additional usermods. None of them included in official releases.

With this information there is no real need for additional information in JSON API as "arch" and "ver" can sufficiently distinguish each of the above (where ESP01 will not allow OTA in any case). And "product" and "brand" are clearly defined in JSON API specification and can be used (by forks or otherwise) to distinguish from official releases as described above.

Note: I am trying to justify inclusion of this (and other additional) field(s) in JSON API and judge the impact it may have on performance and/or operation of WLED. ESP8266 is at the limits regarding the size of JSON. It can no longer support 16 segments due to increased JSON content and adding additional fields will limit it further. That's the reason for reluctance.

Changchung commented 8 months ago

unsubscribe.On Nov 2, 2023 9:14 AM, Blaž Kristan @.***> wrote: you are missing the point. re-read my post. besides, there can be as many different "releases" as there are compile-time options or (more likely) build environments. None of that is happening officially (in the near future). The three release versions for ESP8266 are there just because of 3 different flash sizes (and the 1M version is stripped of OTA capabilities since flash size will not allow it).the only two "official" "optional" releases are Ethernet and Audioreactive (which is technically a usermod but was requested numerous times), all other "releases" feature the same functionality and build options (if there weren't that many GPIOs reserved for Ethernet I would argue to include Ethernet option in every ESP32 build)I may be thinking in a wrong direction, but WLED is not a commercial software it is a DIY project still. So are some forks. As far as I can tell there are official releases with: 2 build options for ESP8266 (with and without OTA)3 build options for ESP32 (plain, with Ethernet, with Audioreactive usermod)1 build option for ESP32-S31 build option for ESP32-S21 build option for ESP32-C3 There are plenty more build environments some of which include variants with PSRAM support, disabled core functionalities (IR, OTA), enabled additional usermods. None of them included in official releases. With this information there is no real need for additional information in JSON API as "arch" and "ver" can sufficiently distinguish each of the above (where ESP01 will not allow OTA in any case). And "product" and "brand" are clearly defined in JSON API specification and can be used (by forks or otherwise) to distinguish from official releases as described above. Note: I am trying to justify inclusion of this (and other additional) field(s) in JSON API and judge the impact it may have on performance and/or operation of WLED. ESP8266 is at the limits regarding the size of JSON. It can no longer support 16 segments due to increased JSON content and adding additional fields will limit it further. That's the reason for reluctance.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: @.***>

jamesmyatt commented 8 months ago

Thanks @blazoncek. I'm thinking about WLED as the centre of a community, rather than as a personal DIY project or a commercial product.

I did get these points, but one of the main lessons I've learned in software is to never assume that things won't change. So I'm worried that, while today you can OTA upgrade any of the ESP8266 official builds with the "ESP8266" build, that won't always be the case, just as today you already can't identify which ESP32 build to use for OTA from just the "arch" value, since different builds have different features. There's also no guarantee that there won't be other future official releases that are non-interchangeable. So if there's a solution that helps the community today handle the new builds released with v0.14.0 as well prepare for the future, then I think it's worth pursuing.

Furthermore, "explicit is better than implicit", so there's no point in trying to guess which build can be used, if it's possible for the device to tell you exactly. I think it's this guessing that bugs me most.

On adding to the JSON API, I agree that this is a big issue: probably the only one that really matters. I get that you don't really want to add anything to the /json endpoint if at all possible, since this response is already pretty big, and changing the API is a major undertaking. My view is that this is valuable information for users and external applications, more so than vid and core, and should be added somewhere in the API (and to the /update page). In fact, I'd argue that build ID is insufficient without the release name (or build target or whatever). But this is all just my view, and I'm happy for the maintainers to say they don't agree.

So I'm wondering about the following potential solutions. I'm not sure whether any of them are feasible or permissible, but I thought I'd write them anyway.

  1. Modify the product field to include the suffix for the optional official builds, e.g. "FOSS_Ethernet" and "FOSS_audioreactive". This might be the minimum change to avoid guessing between incompatible official builds. Also, otherwise, what is this field for?
  2. Similar to (1), but modify the vid field.
  3. Replace vid and core with a field that has the release name. Although, this is probably going to need more bytes than those values combined.
  4. Add an API endpoint that's separate from the /json one with all of the static information about the firmware, i.e. everything that's currently on the /update page, including the releases URL and the release name. This would probably also allow you to move some fields from /json/info to this endpoint, like vid and core.

What do you think?

jamesmyatt commented 4 months ago

OK. Thinking about this more, I think that (1) is actually the better solution, i.e. that the "product" field should be what's used to differentiate between the different official builds, and that field is already in the JSON API. Otherwise, what's the point of the field?

jamesmyatt commented 4 months ago

With the release of v0.14.2 (https://github.com/Aircoookie/WLED/releases/tag/v0.14.2), this issue needs re-raising. There's now another official build, which is the 160MHz ESP8266 one, and this one is very different for OTA and can result in a failed OTA update. I think this makes it harder to justify not being able to determine exactly which build was used. @blazoncek

blazoncek commented 4 months ago

Please make a PR with relevant changes if you think this is so important.

jamesmyatt commented 4 months ago

@blazoncek Are you saying that all of my proposed solutions are acceptable and that all it requires is a PR to move this forwards? Or are you just being rude?

blazoncek commented 4 months ago

My intention is never to be rude, though it sometimes appears so. I was serious if you can figure out the way to accomplish it.

jamesmyatt commented 4 months ago

I've made a PR for using the product name field, now https://github.com/Aircoookie/WLED/pull/3750 has been merged. Seems like the simplest solution and most likely to be acceptable. Separate PR will be required for any changes to the Web UI.

jamesmyatt commented 3 months ago

Based on https://github.com/Aircoookie/WLED/commit/aa970d6ca5773affe33a1c5a322bfe359802e048, I think I'm just going to add a field in the info for WLED_RELEASE_NAME.

blazoncek commented 2 months ago

Available since 674481f0d1d74c7e83de006a752e71cef48da451

jamesmyatt commented 2 months ago

Thank you ❤️