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

Requested pull request.. #9

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

oskarirauta
Copy link

Here you go - hey, I made the changes to setup.py that you requested.....

Oskari Rauta and others added 6 commits November 9, 2015 10:59
 - Update version
 - Remove platforms from the list which I did not upgrade
- Update version string
- Remove support for darwin, freebsd and win32 platforms
 - Add some new definitions
 - Rename function
 - Insert documentation
Update code to v1.5 of this fork
energy_full_filename = 'energy_full'
elif os.path.isfile(os.path.join(supply_path, 'energy_now')) and os.path.isfile(os.path.join(supply_path, 'current_now')) and os.path.isfile(os.path.join(supply_path, 'energy_full')):
energy_now_filename = 'energy_now'
power_now_filename = 'power_now'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be 'current_now'?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! My bad.. Fixed at: https://github.com/oskarirauta/Power/blob/master/power/linux.py

On 9.11.2015, at 22:11, Justin Buchanan [email protected] wrote:

In power/linux.py #9 (comment):

  •                energy_full = float(energy_full_file.readline().strip())
    
  •                return energy_full, energy_now, power_now
    
  •    energy_now_filename = 'charge_now'
    
  •    power_now_filename = 'current_now'
    
  •    energy_full_filename = 'charge_full'
    
  •    if not os.path.isfile(os.path.join(supply_path, energy_now_filename)) or not os.path.isfile(os.path.join(supply_path, power_now_filename)) or not os.path.isfile(os.path.join(supply_path, energy_full_filename)):
    
  •        if os.path.isfile(os.path.join(supply_path, 'energy_now')) and os.path.isfile(os.path.join(supply_path, 'power_now')) and os.path.isfile(os.path.join(supply_path, 'energy_full')):
    
  •            energy_now_filename = 'energy_now'
    
  •            power_now_filename = 'power_now'
    
  •            energy_full_filename = 'energy_full'
    
  •        elif os.path.isfile(os.path.join(supply_path, 'energy_now')) and os.path.isfile(os.path.join(supply_path, 'current_now')) and os.path.isfile(os.path.join(supply_path, 'energy_full')):
    
  •            energy_now_filename = 'energy_now'
    
  •            power_now_filename = 'power_now'
    
    Should this be 'current_now'?


Reply to this email directly or view it on GitHub https://github.com/Kentzo/Power/pull/9/files#r44324395.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. Thanks...

fixed a mistype, thank you Justin Buchanan.
Added maintainer.
Added maintainer
@oskarirauta
Copy link
Author

Updated..

@Kentzo
Copy link
Owner

Kentzo commented Nov 10, 2015

@oskarirauta Why did you need to remove implementation for other platforms?

@Kentzo
Copy link
Owner

Kentzo commented Nov 10, 2015

If you want to exclude them for packaging, it should be possible by modifying setup.py to read some external variable and exclude inclusion of unneeded files. So there won't be any need for multiple forks.

@oskarirauta
Copy link
Author

I excluded them because I worked only with Linux version, you see, I changed it fundamentally, it is no longer "compatible" with previous releases since I "renamed" one of key functions. And even return no longer is compatible.
This is why I made a version bump.

Second, the reason for excluding: I only worked with Linux, since that was only platform I at the moment can test (although, I do have OS X platform and Window 10 "available" but at the moment, they are utilized in a way that I'd better not to tamper with them). To tell you the truth - I didn't even look at them, since I can't provide any testing. Tell you what, if you want to make an "official" v1.5 based on my work; I am more than happy, since I provided this for another project (adeskbar update 0.5.1 -> 0.5.2, also on my repository, battery applet uses this module).

But unfortunately I am not able to provide similar updates for another platforms, so I cannot fulfill that.
If other platforms receive same information, you could update them and use my math, or even better, see if you can make it even shorter code, since it's no doubt, that you are far more better with Python than me, after all, this is 3rd time in my whole life that I use python (ofcourse, I've used it more, but now we talk about creating or changing something)..

So I kinda excluded them because I felt there was no choice since I cannot provide the same for other platforms.

Edit: Err.. when I told I have windows 10 and OS X available, they cannot be used for this, since they don't utilize a battery.

@oskarirauta
Copy link
Author

OS X(darwin) version holds these keys that possibly can be used:
status:
kIOPSACPowerValue
kIOPSPowerSourceStateKey
kIOPSIsPresentKey
kIOPSIsChargedKey
kIOPSIsChargingKey
kIOPSIsFinishingChargeKey
time_remaining:
kIOPSTimeToEmptyKey
kIOPSTimeToFullChargeKey
capacity:
kIOPSCurrentCapacityKey
kIOPSDesignCapacityKey

But I have no idea how these work and what they contain so cannot say how to use the information.
I didn't find anything similar for freebsd or win32 from source.

@Kentzo
Copy link
Owner

Kentzo commented Nov 11, 2015

@oskarirauta It should be possible to extend version for linux with functions you need while preserving common interface and logic this package has and making it compatible with modern version of linux.

If you need some more methods for linux only, just add them to linux.py

@Kentzo
Copy link
Owner

Kentzo commented Jan 10, 2016

I'm willing to merge this PR if anyone can make it so it won't break implementations for other platforms.

@kootenpv
Copy link

It's more the situation that the module won't work in the future for anyone...

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.

4 participants