miskcoo / ugreen_leds_controller

An LED Controller for UGREEN's DX/DXP NAS Series
179 stars 26 forks source link

unRaid #10

Closed werty1st closed 5 months ago

werty1st commented 5 months ago

The ugreen_leds_cli is working on unraid.

I would like to build the kernel module. But I have zero XP with unRaid and slackware.

I will try to load a slackware docker image matching unRaids kernel version and build the module.

miskcoo commented 5 months ago

Hi, if building the kernel module is difficult (and if you have time), you can also try using the programs in the userspace-daemon branch, and check the "Build & Usage" section therein.

I didn't merge this branch to master since I don't have enough time to test it; and it doesn't support blinking netdev under network io.

Basically, you can switch to that branch with git checkout userspace-daemon, and then

ich777 commented 5 months ago

@miskcoo are you okay that I build a plugin for Unraid from your repo and build the modules so that users can easily use it on Unraid? I already have a toolchain that does most of the heavy lifting and auto compiles everything for each new Unraid version.

I will then push it to the Unraid Community Applications where users can easily make use of the driver.

Sorry for the maybe dumb question but I would need to build the cli tool and the Kernel module correct?

werty1st commented 5 months ago

@ich777 its written somewhere that the cli tool and the kernel module would get in conflict with each other. so you can do both but they do not require each other.

how do you build the modules? the developer documentations and tools i found so far are kind of outdated. how are updates handled? does unraid contain something like dkms?

miskcoo commented 5 months ago

@miskcoo are you okay that I build a plugin for Unraid from your repo and build the modules so that users can easily use it on Unraid?

I will then push it to the Unraid Community Applications where users can easily make use of the driver.

I am ok with that if you could note the source in the plugin's description. But if there's no such place, that's okay too.

Including the kernel modules and the scripts directory are sufficient, since the cli tool is conflict with that.

ich777 commented 5 months ago

how do you build the modules? the developer documentations and tools i found so far are kind of outdated. how are updates handled? does unraid contain something like dkms?

I have a special toolchain (and build process for that which involves a Jenkins, VM, Docker and a really complicated script) for that and I work closely together with the developers from Unraid to deliver updates so that driver packages are available when a new Unraid version drops. The updates are actually handled by the plugins themselves and if one updates to a newer Unraid version the new driver package will be downloaded ahead of time so that it is available when you boot into the new Unraid version.

I am ok with that if you could note the source in the plugin's description. But if there's no such place, that's okay too.

Sure thing, I always include the source repo since you are the main developer and I only made the plugin for it, examples: https://github.com/ich777/unraid-it87-driver https://github.com/ich777/unraid-nct6687-driver

Do you have a donation link somewhere so that I can link it in the plugin?

BTW, do you know if the driver will also compile against a newer Kernel since the next Unraid version will use either Kernel 6.7.x+

miskcoo commented 5 months ago

@ich777

Do you have a donation link somewhere so that I can link it in the plugin?

There are no donation link.

do you know if the driver will also compile against a newer Kernel since the next Unraid version will use either Kernel 6.7.x+

Yes, I have tried it on 6.8.9, and as far as I know, it supports >= 5.15.

ich777 commented 5 months ago

Yes, I have tried it on 6.8.9, and as far as I know, it supports >= 5.15.

Give me a bit please, I will try to build it for a few different versions and let you know if it compiles correctly.

ich777 commented 5 months ago

@miskcoo I'm happy to report that the module compiles against Kernel 6.8.12: grafik

Another question, are you also using Unraid? How are the scripts to use? <- sorry again for the dumb question but I don't have such a NAS.

Would one need to run them on boot or is this something that users have to set up manually?

miskcoo commented 5 months ago

@ich777

I am not using Unraid, but if it supports systemd, you can just copy the aforementioned scripts to /usr/bin and enable the service ugreen-ledmon@<netdev_name>.service (the configuration is in scripts).

miskcoo commented 5 months ago

@ich777

  • The user should first run scripts/ugreen-probe-leds at boot;
  • then run scripts/ugreen-netdevmon <netdev_name> to set the netdev LED's blinking when the NIC <netdev_name> is active (so this can be regarded as a parameter); this script will exit;
  • and finally run scripts/ugreen-diskiomon to let the disk LEDs to blink when corresponding disk is active; this script won't exit, it should be kept running.

I am not using Unraid, but if it supports systemd, you can just copy the aforementioned scripts to /usr/bin and enable the service ugreen-ledmon@<netdev_name>.service (the configuration is in scripts).

Forget to mention that, including the i2c-dev, ledtrig-oneshot and ledtrig-netdev modules is necessary.

ich777 commented 5 months ago

@miskcoo understood, I think I will make the base plugin ready that installs the modules since this will probably take me one or two days. After that I will try to implement the scripts into the plugin since this will need some testing.

Unraid doesn't support systemd because it's based on Slackware it uses rc.d however I can do that all when installing the plugin (this is basically done on every boot from Unraid and on the first installation from the plugin).

Forget to mention that, including the i2c-dev, ledtrig-oneshot and ledtrig-netdev modules is necessary.

I know the i2c-dev module because it's built into the base OS from Unraid, however where do I get ledtrig-oneshot and ledtrig-netdev? Are these in tree modules because they aren't shipped with Unraid by default.

ich777 commented 5 months ago

Nevermind, I found it: ledtrig-oneshot: https://cateee.net/lkddb/web-lkddb/LEDS_TRIGGER_ONESHOT.html ledtrig-netdev: https://cateee.net/lkddb/web-lkddb/LEDS_TRIGGER_NETDEV.html

ich777 commented 5 months ago

@miskcoo just to keep you updated, I already integrated your repository into my built toolchain. I also created a repository here but I haven't created a plugin file yet and didn't compile a plugin package yet. Let me know if this repository is okay for you.

Unraid is missing ledtrig-oneshot and ledtrig-netdev but it will be included in the next build from Unraid so that everything will work correctly.

About the scripts: I plan to integrate the probe script directly into the plugin itself, similar to what you are doing in ugreen-probe-leds About the two other scripts, I plan them to start after the plugin is fully installed in the background, that should be enough from what I see. In terms of the ugreen-diskiomon script you have nothing to configure if I see that right, for the ugreen-netdevmon you have to specify the interface correct?

miskcoo commented 5 months ago

@ich777 that sounds good, and you are right ugreen-netdevmon needs to specify the interface.

ich777 commented 5 months ago

@miskcoo I've gone now through the scripts and I think it will be best to implement ugreen-netdevmon directly into the plugin since it runs only once on boot and I can pass through the interface name directly from the config that is set in Unraid.

About the ugreen-diskiomon script I have a few questions and optimizations suggestions:

  1. How much CPU time does the script use? 2. CHECK_SMART should be defined as: ${CHECK_SMART:=true} so that if one sets it to false as environment variable that the value is kept. 3. What do you think about splitting out the check disk health in it's own while loop that runs in the background every fixed 360 seconds to check for smart errors so you can get rid of loop_cnt and maybe rename smart_poll_interval and make it user definable like CHECK_SMART above? This code:
        if [[ "$(cat /sys/class/leds/$led/color)" = "255 0 0" ]]; then
            continue;
        fi

    Checks if the slot is empty if I understand that correct, wouldn't that maybe be a race condition if someone puts in a drive that the leds never would light up? 4. The last loop is set to only sleep 0.1 seconds what do you think about increasing the time to 0.5 seconds or even 1 second? I think that should be also enough for most users and will not hit the CPU so hard. 5. Where is the script: /usr/bin/zpool-leds.sh?

I can submit a PR so that you can go through the changes that I would like to implement if you want to review it.

I now pushed a PR #12 , please let me know what you think of the changes.

miskcoo commented 5 months ago

@ich777

How much CPU time does the script use?

When using sleep 0.1s, it takes about 11s CPU time per minutes in N6005, which looks inefficient. I have made an adjustment that reduces the CPU time to 4.2s.

I may implement this loop using a more efficient language later. I think we can increase the interval from 0.1s to 0.2s or 0.5s by default at this moment.

ich777 commented 5 months ago

@miskcoo I think 0.5 or even 1 second would be sufficient enough for most users.

From what I saw if you put in another drive in the NAS it wouldn't pick up the new drive how it's implemented now however that's not that important for now.

Thanks for your answers and help. :)

Let me know how the changes from my PR impact everything too, really curious.

miskcoo commented 5 months ago

@ich777 You are right. In the current implementation, inserting a new disk requires restarting the script so that it can rebuild the disk mapping.

I think the new version is nearly equivalent to the original version, and removing the existence check of /sys/class/leds/*/color does not affect the results -- it is redundant.

ich777 commented 5 months ago

@miskcoo thank you very much for implementing the changes, it helps a lot! Really much appreciated!

The plugin is now done and I will try to find a few users to test the plugin when the new public beta is released.

@werty1st do you want to close this issue or leave it open for now until the public beta is released? If something is not working in terms of the plugin I think it would be better to continue the conversation on the Unraid forums or in the GitHub repository from the plugin.

Oh I completely forgot you can have a complete build environment, just head over to this repository.

Plugin page preview on Unraid: grafik

PsychoMnts commented 5 months ago

Hi @ich777 amazing work!

I have an DXP6800PRO with Unraid. I can test if you like.

ich777 commented 5 months ago

I have an DXP6800PRO with Unraid. I can test if you like.

Please see this post, we have to wait until a public beta is released with the necessary Kernel modules included.

miskcoo commented 5 months ago

Hi @ich777, I just want to let you know that the PR #15 fixed an issue of mapping from disks to LED, and added a config file in /etc/ugreen-leds.conf (see scripts/ugreen-leds.conf for an example). If you want to include them, I think the changes would be

ich777 commented 5 months ago

@miskcoo thanks for the heads up, I'll have to look into this on Monday because I have a busy weekend, but I already just took a quick look at the changes.

About the ugreen-netdevmon, wouldn't it make much more sense to put it also in the ugreen-diskiomon and/or rename the ugreen-diskiomon to something different like ugreen-leds since the LEDs from the Disk I/O and the Network need to be updated anyway and let them both run in the same last while loop?

I really like the idea with /etc/ugreen-leds.conf but I have to stick to /boot/config/plugins/ugreenleds-driver/settings.cfg since the rootfs on Unraid is not persistent, I mean sure it could be also done that a settings file is created on the boot device and then symlinked to /etc/ugreen-leds.conf.

Was the old way of setting the led color for the Network device not sufficient enough or was this just a static color? BTW, I saw you maybe made a copy/paste mistake here.

miskcoo commented 5 months ago

wouldn't it make much more sense to put it also in the ugreen-diskiomon

That is a good idea, but I think I will keep them separated at this moment.

Was the old way of setting the led color for the Network device not sufficient enough or was this just a static color?

The old way is sufficient so you can also ignore it. It is not a static one. The netdev's blinking is done by the trigger provided by the ledtrig-netdev module -- it is also more efficient.

The new version only changes the color based on the link speeds, and whether we can ping the gateway. Maybe there aren't many people would need them.

BTW, I saw you maybe made a copy/paste mistake here.

Thanks!

ich777 commented 5 months ago

That is a good idea, but I think I will keep them separated at this moment.

No worries, I'm fine with that too... :) The new Unraid beta isn't out yet but it could drop any time (I think in the next few days to be a bit more precise) and I really want to integrate as much as possible so that it reflects your repository and work that you've done.

I will take a closer look at the changes on Monday and report back, do you maybe have a way of communicating through Discord or Matrix/Synapse?