Could not load battery status file

72siddharth commented 3 years ago

The docs state that cnx reads battery info from /sys/class/power_supply/BAT0/ but my device doesn't seem to have this directory. Instead a /sys/class/power_supply/BAT1/ directory exists with all the battery info. So when I use: cnx_add_widget!(cnx, Battery::new(&cnx, attr.clone(), Color::red())); an error is thrown:

Error: Os { code: 2, kind: NotFound, message: "No such file or directory" }

Could not load value from battery status file: charge_full

VuiMuich commented 3 years ago

I believe the Linux specific implementation has been withdrawn from the Widgets/battery.rs.

Probably due to unresolved issues?

psibi commented 3 years ago

I have added Linux implementation back here: https://github.com/mjkillough/cnx/pull/37

The MR still needs some cleaning, I will merge this once I find some spare time.

psibi commented 3 years ago

@72siddharth I just merged the MR to master. Are you able to reproduce the issue with master branch ?

72siddharth commented 3 years ago

@psibi The bar runs but the issue persists in a new form. The battey widget doesn't show up and it's printed Error from widget 6: Could not load value from battery status file: charge_full

72siddharth commented 3 years ago

I also noticed another issue. The volume doesn't update real time. The volume displayed is the volume of the system when the bar was started, and it doesn't update with any changes

psibi commented 3 years ago

Thanks for the feedback! I haven't tested the battery thing yet as I don't run cnx on my laptop as I don't use it frequently these days.

The volume displayed is the volume of the system when the bar was started, and it doesn't update with any changes

That is strange as I can tell you that it works for me. But I'm not that surprised as it's the only module currently which I'm not that happy about. Some questions:

Did the volume widget work for you before ?
Are you getting any messages from cnx when you are updating the volume ?

72siddharth commented 3 years ago

Thanks for your quick response

   Did the volume widget work for you before ?
Previously the bar ran but the position for volume widget was empty Are you getting any messages from cnx when you are updating the volume ? Nope, cnx doesn't seem to notice the volume change or it doesn't check the volume after the first check, if that makes any sense

psibi commented 3 years ago

Previously the bar ran but the position for volume widget was empty

What do you mean by volume widget was empty ? You mean it didn't show the volume before ? You mean in the current behaviour: it's showing volume but it doesn't get updated ?

72siddharth commented 3 years ago

You mean it didn't show the volume before ?

Yea, that's what I meant. Now the volume widget shows up but doesn't update.

psibi commented 3 years ago

@72siddharth I can reproduce the issue at my side and will have a fix soon. What is the output of this in your system:

ls /sys/class/power_supply/BAT1/

psibi commented 3 years ago

@72siddharth I have created a PR which fixes the battery widget for Linux: https://github.com/mjkillough/cnx/pull/43

You can optionally now specify the battery path name:

    let battery = Battery::new(attr.clone(), Color::red(), Some("BAT1".into()), Some(battery_render));

Can you give it a try and see if that fixes it for you ?

72siddharth commented 3 years ago

@72siddharth I can reproduce the issue at my side and will have a fix soon. What is the output of this in your system:

ls /sys/class/power_supply/BAT1/

$ ls /sys/class/power_supply/BAT1/
alarm           model_name
capacity        power
capacity_level      present
charge_full     serial_number
charge_full_design  status
charge_now      subsystem
current_now     technology
cycle_count     type
device          uevent
hwmon2          voltage_min_design
manufacturer        voltage_now

psibi commented 3 years ago

@72siddharth Thanks, that LGTM and I think my new above linked PR should fix the issue for you. Give it a go and let me know how it goes.

72siddharth commented 3 years ago

@psibi So I tried the PR, unfortunately it didn't fix the issue in it's current state and I got this

Error from widget 7: Could not load value from battery status file: current_now

But as it turns out this was a really helpful error message, so I checked .../BAT1/current_now and this is what cat results in

cat: /sys/class/power_supply/BAT1/current_now: No such device

So I fiddled around with the battery fetching code and switched "current_now" with "charge_now" within the load_value function, and behold it works perfectly.

psibi commented 3 years ago

@72siddharth Thanks, I will investigate more about it to understand this further. From a quick google the above issue you are facing looks like a BIOS issue: https://bugzilla.kernel.org/show_bug.cgi?id=83411

72siddharth commented 3 years ago

@psibi You're right, seems to be more of a firmware issue. Thank you, the issue seems to be solved by replacing "current_now" with "charge_now" although I do not understand the overall implications of this. And unfortunately the volume widget problem still persists, of not updating with changes in volume.

psibi commented 3 years ago

@72siddharth Thanks, yeah I'm looking to see if there a better way to solve this. Can you open a new issue for the volume widget problem ?

psibi commented 3 years ago

@72siddharth I have updated the MR. Can you give that a try ?

72siddharth commented 3 years ago

The battery widget now works perfectly. Thank you very much.

mjkillough / cnx

Could not load battery status file #38