search

hudra0 / qosmate

GNU General Public License v3.0
41 stars 7 forks source link

refactor with OpenWrt style #9

Open morytyann opened 3 weeks ago

morytyann commented 3 weeks ago
  1. add option qosmate.global.status to show current status
  2. qosmate.global.enabled now works properly
  3. remove tc libs because it already be depended in Makefile
  4. revise Makefile
morytyann commented 3 weeks ago

If you want to merge this, you will also need to merge https://github.com/hudra0/luci-app-qosmate/pull/7 .

hudra0 commented 2 weeks ago

Thank you for the refactor! However, adding qosmate.global.status in the config might be redundant, as it doesn’t actively represent the service’s real-time status.

Additionally:

Could you clarify the intent behind some of these changes (also the ui changes)? It would help in understanding the potential impacts. Thank you!

morytyann commented 2 weeks ago

Thanks for replying!

it doesn’t actively represent the service’s real-time status.

Yes! But we don't have any procd instances which we can get status, maybe we can get status by check if nftables table exists?

Moving etc/init.d/qosmate to files/qosmate.init doesn’t seem like it follows OpenWrt conventions

image

The removal of the tc distribution files (for netem) might cause issues, as these settings may not be included in the build by default and could lead to non-functional configurations.

Sorry for that, I take it for granted that tc-full provides these files, i will fix it.

As i said, the intent behind of changes is refactor with OpenWrt style:

  1. qosmate.global.enabled should be a switch of our application.
  2. /etc/init.d/qosmate enable or disable, it not means enable or disable the service, it means whether the service will auto start after boot.
  3. we don't need reload/restart service by manual in luci, just add reload trigger for qosmate config and OpenWrt will handle it, this is why i remove all of handleSaveAndApply in luci views.
  4. i think we should distribute the package by ipk files rather than download files seperate, this is why i remove all code about update/download files.
  5. the dependencies of package will always be installed, so i remove the code for check/install dependencies.
  6. i remove the code of preserve config files, because those files are not config files, and we should not do like this, just create a file named qosmate.upgrade, write the paths of files which we need to keep into it and install it to /lib/upgrade/keep.d
brada4 commented 2 weeks ago

Please remove unnecessary firewall4 dependency. It needs conntrack and dscp

morytyann commented 2 weeks ago

Done.