luci icon indicating copy to clipboard operation
luci copied to clipboard

luci-app-advanced-reboot: update to 1.1.1

Open stangri opened this issue 6 months ago • 10 comments

Maintainer: me Compile tested: x86_64, Linksys MX4200v1, OpenWrt 24.10.2 Run tested: x86_64, Linksys MX4200v1, OpenWrt 24.10.2

Description: This is a massive overwrite of the RPCD code from shell script to ucode which in turn brought:

  • update to the device json schema, which is now:
    • more structured
    • no weird names/object in json
    • customizable get, set, save commands per device (still need to be defined in ACL)
    • single partitions array now holding all data for toggling/labels/altMount
    • support for more than 2 bootable partition devices
  • a _device_json_transform.jq file to translate schema from old format to new one for any WIP devices
  • simplification of RPCD script as object can now be returned
  • adjustments to the Javascript to parse new error messages and new RPCD reply structure

I have tested both UBI volumes and dd-based information gathering on alternative partition, I'd still welcome:

  • tests on as many Linksys devices for actual partition toggling (rebooting to alternative)
  • tests for both information gathering and partition toggling on:
    • "d-link,dgs-1210-28"
    • "mercusys,mr90x-v1"
    • "zyxel,nbg6817"

I also gladly welcome any code reviews for both updated javascript and ucode.

stangri avatar Aug 24 '25 02:08 stangri

I've addressed the issues discovered in testing, unless there's any feedback it should be ready to merge.

If @systemcrash @stokito @champtar @Ramon00 or anyone else have time to review and comment on the new code, I'd be much obliged.

stangri avatar Sep 26 '25 00:09 stangri

Perhaps some more comments for each functions which give examples of sane input values - useful when following each step.

systemcrash avatar Sep 29 '25 16:09 systemcrash

Perhaps some more comments for each functions which give examples of sane input values - useful when following each step.

sorry do you mean the RPC functions or all functions in ucode?

stangri avatar Sep 30 '25 05:09 stangri

Perhaps some more comments for each functions which give examples of sane input values - useful when following each step.

sorry do you mean the RPC functions or all functions in ucode?

ucode.

systemcrash avatar Sep 30 '25 13:09 systemcrash

Still working on more comments for still "undocumented" functions as per @systemcrash advise.

stangri avatar Oct 07 '25 22:10 stangri

Still working on more comments for still "undocumented" functions as per @systemcrash advise.

A few that lack comments is OK - but a cursory hint to what most of the functions do is really helpful, especially to reviewers :)

systemcrash avatar Oct 07 '25 22:10 systemcrash

LGTM

systemcrash avatar Oct 07 '25 22:10 systemcrash

@systemcrash updated the ucode with (mostly AI-generated) function descriptions.

I've also tried to adjust the Makefile to generate the devices.json during build and remove no longer needed directories when building the package.

stangri avatar Oct 08 '25 01:10 stangri

ping @stangri

systemcrash avatar Dec 03 '25 20:12 systemcrash

@systemcrash I'll be back mid-December, so hoping to push latest fix(es) and merge between then and X-mas.

stangri avatar Dec 06 '25 20:12 stangri

Still in the pipeline. I believe I need to update PR with some fixes and address @systemcrash questions before it can be merged.

stangri avatar Jan 04 '26 20:01 stangri

[!WARNING]

Some formality checks failed.

Consider (re)reading submissions guidelines.

Failed checks

Issues marked with an :x: are failing checks.

Commit 884840972472e7b430719598fdad99d98edb7b4f

  • :large_orange_diamond: Commit body line 1 is longer than 75 characters (is 93): This is a massive overwrite of the RPCD code from shell scrit to ucode which in turn brought:
  • :large_orange_diamond: Commit body line 5 is longer than 75 characters (is 85): * customizable get, set, save commands per device (still need to be defined in ACL)
  • :large_orange_diamond: Commit body line 6 is longer than 75 characters (is 77): * single partitions array now holding all data for toggling/labels/altMount
  • :large_orange_diamond: Commit body line 8 is longer than 75 characters (is 101):
    • a _device_json_transform.jq file to translate schema from old format to new one for any WIP devices
  • :large_orange_diamond: Commit body line 10 is longer than 75 characters (is 88):
    • adjustments to the Javascript to parse new error messages and new RPCD reply structure
  • :large_orange_diamond: Commit body line 13 is longer than 75 characters (is 110): I have tested both UBI volumes and dd-based information gathering on alternative partition, I'd still welcome:
  • :large_orange_diamond: Commit body line 14 is longer than 75 characters (is 91):
    • tests on as many Linksys devices for actual partition toggling (rebooting to alternative)
  • :large_orange_diamond: Commit body line 20 is longer than 75 characters (is 77): I also gladly welcome any code reviews for both updated javascript and ucode.

For more details, see the full job log.

Something broken? Consider providing feedback.

github-actions[bot] avatar Jan 06 '26 20:01 github-actions[bot]

[!WARNING]

Some formality checks failed.

Consider (re)reading submissions guidelines.

Failed checks

Issues marked with an :x: are failing checks.

Commit de33f18d0f4e6576f82a942b5634cf5fba6b6224

  • :large_orange_diamond: Commit body line 1 is longer than 75 characters (is 93): This is a massive overwrite of the RPCD code from shell scrit to ucode which in turn brought:
  • :large_orange_diamond: Commit body line 5 is longer than 75 characters (is 85): * customizable get, set, save commands per device (still need to be defined in ACL)
  • :large_orange_diamond: Commit body line 6 is longer than 75 characters (is 77): * single partitions array now holding all data for toggling/labels/altMount
  • :large_orange_diamond: Commit body line 8 is longer than 75 characters (is 101):
    • a _device_json_transform.jq file to translate schema from old format to new one for any WIP devices
  • :large_orange_diamond: Commit body line 10 is longer than 75 characters (is 88):
    • adjustments to the Javascript to parse new error messages and new RPCD reply structure
  • :large_orange_diamond: Commit body line 13 is longer than 75 characters (is 110): I have tested both UBI volumes and dd-based information gathering on alternative partition, I'd still welcome:
  • :large_orange_diamond: Commit body line 14 is longer than 75 characters (is 91):
    • tests on as many Linksys devices for actual partition toggling (rebooting to alternative)
  • :large_orange_diamond: Commit body line 20 is longer than 75 characters (is 77): I also gladly welcome any code reviews for both updated javascript and ucode.

For more details, see the full job log.

Something broken? Consider providing feedback.

github-actions[bot] avatar Jan 06 '26 21:01 github-actions[bot]

[!WARNING]

Some formality checks failed.

Consider (re)reading submissions guidelines.

Failed checks

Issues marked with an :x: are failing checks.

Commit c2b717d65c45f0c2d66da98c4556390c8086ca43

  • :large_orange_diamond: Commit body line 1 is longer than 75 characters (is 93): This is a massive overwrite of the RPCD code from shell scrit to ucode which in turn brought:
  • :large_orange_diamond: Commit body line 5 is longer than 75 characters (is 85): * customizable get, set, save commands per device (still need to be defined in ACL)
  • :large_orange_diamond: Commit body line 6 is longer than 75 characters (is 77): * single partitions array now holding all data for toggling/labels/altMount
  • :large_orange_diamond: Commit body line 8 is longer than 75 characters (is 101):
    • a _device_json_transform.jq file to translate schema from old format to new one for any WIP devices
  • :large_orange_diamond: Commit body line 10 is longer than 75 characters (is 88):
    • adjustments to the Javascript to parse new error messages and new RPCD reply structure
  • :large_orange_diamond: Commit body line 13 is longer than 75 characters (is 110): I have tested both UBI volumes and dd-based information gathering on alternative partition, I'd still welcome:
  • :large_orange_diamond: Commit body line 14 is longer than 75 characters (is 91):
    • tests on as many Linksys devices for actual partition toggling (rebooting to alternative)
  • :large_orange_diamond: Commit body line 20 is longer than 75 characters (is 77): I also gladly welcome any code reviews for both updated javascript and ucode.

For more details, see the full job log.

Something broken? Consider providing feedback.

github-actions[bot] avatar Jan 06 '26 23:01 github-actions[bot]

[!WARNING]

Some formality checks failed.

Consider (re)reading submissions guidelines.

Failed checks

Issues marked with an :x: are failing checks.

Commit 66731534d8f8059d53fb7a7a00f70af1fa88ce3b

  • :large_orange_diamond: Commit body line 1 is longer than 75 characters (is 93): This is a massive overwrite of the RPCD code from shell scrit to ucode which in turn brought:
  • :large_orange_diamond: Commit body line 5 is longer than 75 characters (is 85): * customizable get, set, save commands per device (still need to be defined in ACL)
  • :large_orange_diamond: Commit body line 6 is longer than 75 characters (is 77): * single partitions array now holding all data for toggling/labels/altMount
  • :large_orange_diamond: Commit body line 8 is longer than 75 characters (is 101):
    • a _device_json_transform.jq file to translate schema from old format to new one for any WIP devices
  • :large_orange_diamond: Commit body line 10 is longer than 75 characters (is 88):
    • adjustments to the Javascript to parse new error messages and new RPCD reply structure
  • :large_orange_diamond: Commit body line 13 is longer than 75 characters (is 110): I have tested both UBI volumes and dd-based information gathering on alternative partition, I'd still welcome:
  • :large_orange_diamond: Commit body line 14 is longer than 75 characters (is 91):
    • tests on as many Linksys devices for actual partition toggling (rebooting to alternative)
  • :large_orange_diamond: Commit body line 20 is longer than 75 characters (is 77): I also gladly welcome any code reviews for both updated javascript and ucode.

For more details, see the full job log.

Something broken? Consider providing feedback.

github-actions[bot] avatar Jan 07 '26 00:01 github-actions[bot]

@stangri @systemcrash This new luci-app-advanced-reboot seems to be broken on my Linksys MR5500. There are at least 3 issues that I've noticed:

  1. Trying to reboot to alternative partition - clicking "Reboot to this partition" will return an error: Invalid request: number is required and must be numeric., see screenshot: image

  2. The firmware build version is not detected anymore on the current partition, it previously said something like "OpenWrt SNAPSHOT r32598", now there's no more version specified.

  3. The confirmation popup has broken text (missing spaces), i.e. the backslashes in the code to break the long text into separate lines do not work as intended: image

Can this commit be reverted please?

lorand-horvath avatar Jan 10 '26 11:01 lorand-horvath

Paste the JSON output from the browser console produced when you click 'reboot'; its accompanying 'request' and 'response' from the request respective response tab of the POST request.

systemcrash avatar Jan 11 '26 13:01 systemcrash

Paste the JSON output from the browser console produced when you click 'reboot'; its accompanying 'request' and 'response' from the request respective response tab of the POST request.

@systemcrash There's no error in the console. The request/response looks like this:

request: [{"jsonrpc":"2.0","id":5,"method":"call","params":["ff63eb89442e15594fc23d05689c5bbb","luci.advanced-reboot","boot_partition",{"number":"1"}]}]

response:

[
    {
        "jsonrpc": "2.0",
        "id": 5,
        "result": [
            0,
            {
                "error": "INVALID_ARG",
                "detail": "number is required and must be numeric"
            }
        ]
    }
]

lorand-horvath avatar Jan 11 '26 17:01 lorand-horvath

@lorand-horvath what happens if you call from the terminal like so: ubus call luci.advanced-reboot boot_partition '{ "number": "1" }' or ubus call luci.advanced-reboot boot_partition '{ "number": 1 }'?

systemcrash avatar Jan 11 '26 19:01 systemcrash

# ubus call luci.advanced-reboot boot_partition '{ "number": "1" }'
{
        "error": "INVALID_ARG",
        "detail": "number is required and must be numeric"
}
# ubus call luci.advanced-reboot boot_partition '{ "number": 1 }'
Command failed: Invalid argument

lorand-horvath avatar Jan 11 '26 20:01 lorand-horvath

@stangri @systemcrash This is a mess. Please revert the commit until there's an actual, tested, functional version.

lorand-horvath avatar Jan 12 '26 10:01 lorand-horvath

@stangri Care to say something? The package is completely broken. You shouldn't leave it in this state, it's just not fair with respect to the community. @systemcrash Please revert.

lorand-horvath avatar Jan 14 '26 09:01 lorand-horvath

@systemcrash if (req && req.number != null) number = int(req.number); a better option might be: if (req && req.args && req.args.number != null) number = int(req.args.number);

I believe that req.args.number is the key here. After this change it works for two different SOCs.

@stangri Thanks for this version. I can drop my custom patch and just add a json file and the basic functionality work. (for now I'm patching the above, but I'm expecting this to be solved eventually)

ghost avatar Jan 16 '26 06:01 ghost

@lorand-horvath if you can confirm that fixes things for you, I can push it out.

systemcrash avatar Jan 17 '26 22:01 systemcrash

@stangri @systemcrash I have installed the latest version 1.1.1-15 from https://github.com/stangri/luci-app-advanced-reboot/releases and it seems to be working fine (I've only tested rebooting to Alternative partition).

apk add --allow-untrusted luci-app-advanced-reboot-1.1.1-15_openwrt-25.12_noarch.apk
image

lorand-horvath avatar Jan 18 '26 10:01 lorand-horvath

@stangri Have you noticed this error visible in "make defconfig" etc. configuration actions:

WARNING: Makefile 'package/feeds/luci/luci-app-advanced-reboot/Makefile' has a build dependency on 'jq/host', which does not exist

hnyman avatar Feb 02 '26 18:02 hnyman

@hnyman I don't really use full builds, before PR I compile with SDK and rely on CI to figure out any errors. Is jq installed on host?

stangri avatar Feb 02 '26 19:02 stangri

Is jq installed on host?

No idea. And should have no relevancy. But you declared a dependency for that host build to be in the OpenWrt build root, although the jq package itself does not declare a host build. So, there is no proper host build.

https://github.com/openwrt/packages/blob/master/utils/jq/Makefile

There is no host build defined in the Makefile. And for that reason all config evaluations throw a hiccup to your requirement of that jq/host .

Error is visible e.g. in all buildbot builds. https://buildbot.openwrt.org/images/#/builders/23/builds/514/steps/21/logs/stdio

hnyman avatar Feb 02 '26 20:02 hnyman

jq is not available AFAICT on host build. It's available as a package for users tho. That should be removed from host deps. Just gotta jq the JSON before releases :/

systemcrash avatar Feb 03 '26 05:02 systemcrash

jq is not available AFAICT on host build. It's available as a package for users tho. That should be removed from host deps. Just gotta jq the JSON before releases :/

jq on host is very useful tho, I was thinking of merging the luci-app-https-dns-proxy providers files in Makefile same way, so I've created a PR to build it on host and it's linked just above your comment. ;)

stangri avatar Feb 03 '26 19:02 stangri