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

Use product name to identify OTA-compatible releases #3839

Closed
wants to merge 1 commit into from

Conversation

jamesmyatt
Copy link

Aim to make it easier for third-party apps, like https://github.com/frenck/python-wled, to identify which release can be used for OTA updates for official builds without guessing. Non-official builds from WLED source code now have product name = "DIY" by default.

Partially addresses: #3497
Follows from: #3750

More direct solution would be to add to add a new field in the JSON API, but this solution avoid complicating the API.

Official product names:

  • FOSS = basic build
  • FOSS_OC = overclocked build (for ESP8266)
  • FOSS_AR = audioreactive build
  • FOSS_Eth = Ethernet build
  • FOSS_PSRAM = build with PSRAM

@blazoncek
Copy link
Collaborator

blazoncek commented Mar 21, 2024

Just a quick question: Why not reuse WLED_RELEASE_NAME to distinguish variants?
It is unique as far as official builds go.

Reasoning:

If your only objection is "FOSS" being default value for WLED_PRODUCT_NAME (and you propose "DIY") which is exposed in JSON API, then, IMO, it is up to the person doing compile to append WLED_PRODUCT_NAME override into build environment.

If, on the other hand, you want to distinguish "ESP8266" from "ESP8266_160" (which is just a temporary compile until we get sufficient feedback) from within JSON API then add that (WLED_RELEASE_NAME) into JSON API and you have created a reasonable API extension. Otherwise I would argue that there are no functional differences and excluding one or the other from available options would prohibit users from choosing the best option.

I've already tried to explain that you can safely upload (OTA) ESP02 to a device originally flashed with ESP8266 binary (and vice versa) as there are no differences in the binary when doing OTA. The difference is only apparent when performing flashing via USB.
The same holds true for any other build.

EDIT: I was wrong regarding ESP8266. ESP8266 binary includes flash size within it and it will use that even when using OTA.

@jamesmyatt
Copy link
Author

jamesmyatt commented Mar 21, 2024

Thanks @blazoncek

I took 2 main points for our previous discussion:

  1. We should avoid adding to the (main) JSON API if at all possible.
  2. It's only necessary to add enough information to identify any OTA compatible binary. In particular, it's only necessary to distinguish between the 3 ESP32 builds (although there are now PSRAM and overclocked builds now too).

It seemed like making a small modification to the product value was going to be the easiest here, since the combination of arch and product value should be enough to identify a suitable binary for OTA, provided brand == "WLED". Although maybe our assumptions aren't true for that now, if you need to take the flash size into account.

I didn't consider replacing the product value with the release name, but that might be an option. But I assume that there's a purpose in mind and #3497 has just made it configurable at compile time.

I thought it was necessary to separate the overclocked ESP8266 builds from the regular ones, since flashing the wrong one will change the CPU clock and they're experimental and might result in a bricked device. If someone uploaded an overclocked build, then we'd want to OTA to a new overclocked build, but if someone had a regular build, then we'd want to keep the regular build. If the overclocked build is removed in the future, then the OTA logic can just map both product values to the new base build.

Setting the default value to "DIY" means that devices with custom builds should be detected as non-standard easily, and automatic OTA to an official release disabled for those cases.

@softhack007
Copy link
Collaborator

Official product names:

  • FOSS = basic build
  • FOSS_OC = overclocked build (for ESP8266)
  • FOSS_AR = audioreactive build
  • FOSS_Eth = Ethernet build
  • FOSS_PSRAM = build with PSRAM

Actually this scheme might not be sufficient for ESP32-S3. On -S3 there are several possibilities for having PSRAM, and unfortunately each oth these requires a different bootloader and firmware build:

  • -S3 without PSRAM (dio or qio flash)
  • -S3 with "qspi" mode PSRAM (qio flash and qspi PSRAM)
  • -S3 with "opi" mode PSRAM (qio flash and octal PSRAM)
  • (future support) -S3 with "opi_opi" PSRAM (octal flash and octal psram)

@jamesmyatt
Copy link
Author

@softhack007 What's your suggestion? Is this proposal OK for now, but likely to get much more complicated in the future, or is it not OK for now because it's likely to get much more complicated in the future? Would you prefer that the product name matches the release name, or that there's a separate field for the release name? Or something else?

@blazoncek
Copy link
Collaborator

It would be beneficial if this and #3407 be combined in a universal solution.
One additional note/requirement would be to include all (or at least important ones) build flags used (hash or something similar).

@jamesmyatt
Copy link
Author

jamesmyatt commented Mar 28, 2024

The more I think about this, the more I'm convinced that the idea of "OTA-compatible" is inappropriate for the product name. I can get behind "basic", "audio-reactive", "ethernet" and (maybe) "psram" as being different products, with "DIY" for custom builds, since they need different fundamentally hardware and have different capabilities, so would look like different products to a user. But variants around different CPU clocks, flash sizes and io widths are a distraction/red herring as far as "product" is concerned.

For OTA, you might as well store the exact release name rather than trying to guess.

So I'm tempted to make a new PR with a new HTTP API endpoint with all of the static information about the device (i.e. doesn't change without a reboot). And then either (1) abandon this PR or (2) update it to remove the "OC" product name.

Any views? Or are they all good suggestions?

@blazoncek
Copy link
Collaborator

You keep referring to WLED as a product. It is not. It is a hobby software that enables various hardware to display LED animations.
Most of this hardware is still a DIY endeavour and will vary greatly - in components and function.

When WLED is used in commercial products (being @intermittech or @srg74 or @wled-install or any other) it is in their (manufacturer) best interest to provide official builds tailored for their hardware. In such case users can be advised that using non-customized firmware may break functionality of their device (if it may do so). The three mentioned keep their firmware repositories on par with WLED development, but I cannot say for others.

While that being said, I always try to keep compatibility with every hardware I received for testing (and evaluation) as to reduce possibility to brick a device. There are no usermods included in official builds for the reasons of compatibility, though many of them would benefit of (at least) one.

That's also why I want a solution beyond simple #define change. If we are to embrace firmware awareness of its build options it has to be complete - all build flags included including variations like CPU/flash speed.

@jamesmyatt
Copy link
Author

jamesmyatt commented Mar 28, 2024

It wasn't me who chose "brand" and "product" as the names of the fields in the API. But I don't think the semantics are that important. I'm just trying to fit solutions into the existing implementation in a way that minimises work and surprises.

"Aircoookie/WLED" is a named entity that distributes pre-built firmwares, whether it's for fun or profit, and those match specific reference hardware configurations or "products" as a user would understand them.

@jamesmyatt jamesmyatt marked this pull request as draft April 12, 2024 11:51
@blazoncek
Copy link
Collaborator

Screenshot 2024-04-22 at 18 01 47
Screenshot 2024-04-22 at 18 00 20

@jamesmyatt
Copy link
Author

I've replaced this specific PR with #3925. I don't think using WLED_PRODUCT_NAME for the purpose of identifying OTA-compatible builds is a good idea.

@jamesmyatt jamesmyatt closed this Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants