Learning about pull requests.  

The second one is the simpler version using sysctl kern.bootime.  I think 
this is easier than pulling uptime, using a regex to convert to a float.  
As well as reordering to make the order Linux, FreeBSD, MacOS.  

I'd still like to see setup.py pick from one of the choices (auto OS 
detect/menu selection) and just insert the correct routine.  But I think 
getting this first patch in makes my application upgrade path easier.  But 
I also ponder the option of making a simple subroutine in station.py for 
additional OSes selected by the weewx.conf general configuration option.  
Which could also be set by the setup.py task. 

OS='Linux'
#OS='FreeBSD'
#OS='MacOS'


On Monday, December 12, 2016 at 6:29:56 PM UTC-8, Tom Keffer wrote:
>
> ​Hello, Bill
>
> Unfortunately, I could not get your code to work.
>
> It has an indentation error, line 131.
>
> Your code tries importing ctypes. Because ctypes is part of the Python 
> standard library, this always succeeds. Unfortunately, for non-BSD systems, 
> this logic branch leaves dylib undefined.
>
> I'm not sure what you mean by "port idea for FreeBSD." If you mean a 
> separate copy of station.py, I'm against the idea. Since its start, a 
> design principle of weewx has always been "one code base."
>
> The easiest way to do a pull request is through GitHub. Fork the weewx 
> repository <https://github.com/weewx/weewx>, make your changes, then ask 
> for a pull request. This makes it very easy to see what the differences 
> are, review the changes, then merge them into the code base.
>
> -tk
>  
>
> On Mon, Dec 12, 2016 at 5:32 PM, Bill Richter <bill.g...@gmail.com 
> <javascript:>> wrote:
>
>> I looked at the documentation again and providing patches was a topic I 
>> didn't have success locating.  Google had the send a diff to the mailing 
>> list for review but that was vintage 2012.  If could expand a bit on how to 
>> make a pull request?  My SVN repo isn't public. But I can attach the file.
>>
>>
>> I thought the easy fix is change the order of the tests to most popular 
>> to least.  But during install the script could ask and here are three 
>> choices.  It's easy for the RPM method since the OS is known.  But it would 
>> be also nice if setup.py could figure it out, but at the same time, putting 
>> it in the weewx.conf is also pick one of these choices also making it 
>> easy.  
>>
>>
>> That's beyond my current python skill set, but I also didn't want to go 
>> further without getting better direction/suggestions. Getting the basic 
>> patch 
>>
>> in keeps my upgrade path simple. But at the same time, if there was a 
>> port for FreeBSD, this would solve those what OS selection questions too.  
>> The more I think about it the more I like the port idea for FreeBSD. 
>>
>> On Sunday, December 11, 2016 at 12:30:45 PM UTC-8, Bill Richter wrote:
>>>
>>> The inline diff works for me.  I'm not a diehard python coder.  I'll 
>>> like to have this touched up a bit.  While this works, it might not be 
>>> ideal.  I don't like the hard coded library path needed for BSD. 
>>>
>>> * What I'd like is to have the OS installed as a weewx.conf option.  
>>> Then the station.py code could get the OS version and use the correct 
>>> library path and not have keep testing which OS is being used.
>>>      
>>> * Make setup.py find the correct OS and insert the correct code again, 
>>> keeping station.py from checking on every execution.
>>>      
>>> * Failing 1 or 2, perhaps the order should be Linux, Freebsd, MacOS?   
>>> This keeps is close to the current configuration.
>>>      
>>> * I think it might be simple to have Linux pull data from /proc as 
>>> opposed to using the same code as FreeBSD and executing based on the OS 
>>> detected.  But I think option 1 and 2 would still be nice.  Just install 
>>> the correct code in a stub in station.py.
>>>      
>>> * I only tested this on FreeBSD.  I'm not sure all the 
>>> necessary/recommended error recovery is in place.  I think the existing 
>>> code will return NA if one of the three matches isn't found.
>>>
>>> # diff --context station.py.orig station.py.new
>>> *** station.py.orig    Tue Dec  6 10:23:34 2016
>>> --- station.py.new    Sun Dec 11 12:20:08 2016
>>> ***************
>>> *** 8,13 ****
>>> --- 8,14 ----
>>>   
>>>   import weeutil.weeutil
>>>   import weewx.units
>>> + import os, sys, datetime
>>>   
>>>   # For MacOS:
>>>   try:
>>> ***************
>>> *** 100,121 ****
>>>       @property
>>>       def os_uptime(self):
>>>           """Lazy evaluation of the server uptime."""
>>> -         # Get the OS uptime. Because this is highly operating system 
>>> dependent, several
>>> -         # different strategies may have to be tried:
>>>           os_uptime_secs = None
>>> !         try:
>>>               # For Linux:
>>> !             os_uptime_secs = 
>>> float(open("/proc/uptime").read().split()[0])
>>> !         except (IOError, KeyError):
>>> !             try:
>>>                   # For MacOs:
>>> !                 os_uptime_secs = CACurrentMediaTime()
>>> !             except NameError:
>>> !                 pass
>>> ! 
>>> !         return weewx.units.ValueHelper(value_t=(os_uptime_secs, 
>>> "second", "group_deltatime"),
>>> !                                        formatter=self.formatter,
>>> !                                        converter=self.converter)
>>>   
>>>       def __getattr__(self, name):
>>>           # This is to get around bugs in the Python version of 
>>> Cheetah's namemapper:
>>> --- 101,146 ----
>>>       @property
>>>       def os_uptime(self):
>>>           """Lazy evaluation of the server uptime."""
>>>           os_uptime_secs = None
>>> !     try:
>>> !             import ctypes
>>> !             class timespec(ctypes.Structure):
>>> !                  _fields_ = [
>>> !                 ('tv_sec', ctypes.c_long), ('tv_nsec', ctypes.c_long)
>>> !                  ]
>>> ! 
>>> !             if sys.platform.startswith('freebsd'):
>>> !                 CLOCK_MONOTONIC = 4# see < time.h >
>>> !                 dylib = 'libc.so.7'
>>> !             #else :
>>> !             #    sys.platform.startswith('linux')
>>> !             #    CLOCK_MONOTONIC = 1# see < linux / time.h >
>>> !             #    dylib = 'librt.so.1'
>>> ! 
>>> !             syslib = ctypes.CDLL(dylib, use_errno = True)
>>> !             clock_gettime = syslib.clock_gettime
>>> !             clock_gettime.argtypes = [ctypes.c_int, 
>>> ctypes.POINTER(timespec)]
>>> !             t = timespec()
>>> ! 
>>> !             if clock_gettime(CLOCK_MONOTONIC, ctypes.pointer(t)) != 0:
>>> !                 errno_ = ctypes.get_errno()
>>> !              raise OSError(errno_, os.strerror(errno_))
>>> !             os_uptime_secs = t.tv_sec + t.tv_nsec * 1e-9
>>> !     
>>> !     except ImportError:
>>> !             try:
>>>               # For Linux:
>>> !                 os_uptime_secs = 
>>> float(open("/proc/uptime").read().split()[0])
>>> !             except (IOError, KeyError):
>>> !                 try:
>>>                   # For MacOs:
>>> !                  os_uptime_secs = CACurrentMediaTime()
>>> !                 except NameError:
>>> !                  pass
>>> ! 
>>> !         return weewx.units.ValueHelper(value_t=(os_uptime_secs, 
>>> "second", "group_deltatime"), 
>>> !                                         formatter=self.formatter,
>>> !                                         converter=self.converter)
>>>   
>>>       def __getattr__(self, name):
>>>           # This is to get around bugs in the Python version of 
>>> Cheetah's namemapper:
>>>
>>
>

Reply via email to