livysdad27 / tempestWS

Weatherflow Tempest WebSocket Driver
GNU General Public License v3.0
5 stars 0 forks source link

`closePort` references undefined variable #1

Closed tkeffer closed 2 years ago

tkeffer commented 2 years ago

Your definition of closePort() looks like this:

    def closePort(self):
        # Shut down the events if the driver is closed.
        ws.send('{"type":"listen_rapid_stop",' + ' "device_id":' + self._tempest_device_id + ',' + ' "id":"listen_rapid_stop"}')
        resp = ws.recv()
        loginf(resp)

        ws.send('{"type":"listen_stop",' + ' "device_id":' + self._tempest_device_id + ',' + ' "id":"listen_stop"}')
        resp = ws.recv()
        loginf(resp)

Notice how the variable ws is not defined before using it. This results in a NameError exception

The problem is that ws is created in getLoopPackets(), but never stored. I would suggest changing this line

        ws = create_connection(self._ws_uri)

to this

        this.ws = create_connection(self._ws_uri)

with corresponding changes wherever ws is used.

Another issue: the driver does not take advantage of the new WeeWX V4 logging. Fixing this is simple. Change this

# These are some handy syslog functions. 
def logmsg(level, msg):
    syslog.syslog(level, 'tempestAPI: %s' % msg)

def logdbg(msg):
    logmsg(syslog.LOG_DEBUG, msg)

def loginf(msg):
    logmsg(syslog.LOG_INFO, msg)

def logerr(msg):
    logmsg(syslog.LOG_ERR, msg)

to this:

try:
    # Test for new-style weewx logging by trying to import weeutil.logger
    import weeutil.logger
    import logging
    log = logging.getLogger(__name__)

    def logdbg(msg):
        log.debug(msg)

    def loginf(msg):
        log.info(msg)

    def logerr(msg):
        log.error(msg)

except ImportError:
    # Old-style weewx logging
    import syslog

    def logmsg(level, msg):
        syslog.syslog(level, 'tempestAPI: %s:' % msg)

    def logdbg(msg):
        logmsg(syslog.LOG_DEBUG, msg)

    def loginf(msg):
        logmsg(syslog.LOG_INFO, msg)

    def logerr(msg):
        logmsg(syslog.LOG_ERR, msg)
livysdad27 commented 2 years ago

Thx Tom!

I'm on holiday/vacation and was silly to add the close port code without thoroughly testing it. Thx for the tips.

Logging update was on my to-do also. Appreciate the help.

I'll have a keyboard again next week.

Billy

On Sun, Jul 24, 2022, 5:11 PM Tom Keffer @.***> wrote:

Your definition of closePort() looks like this:

def closePort(self):
    # Shut down the events if the driver is closed.
    ws.send('{"type":"listen_rapid_stop",' + ' "device_id":' + self._tempest_device_id + ',' + ' "id":"listen_rapid_stop"}')
    resp = ws.recv()
    loginf(resp)

    ws.send('{"type":"listen_stop",' + ' "device_id":' + self._tempest_device_id + ',' + ' "id":"listen_stop"}')
    resp = ws.recv()
    loginf(resp)

Notice how the variable ws is not defined before using it. This results in a NameError exception

The problem is that ws is created in getLoopPackets(), but never stored. I would suggest changing this line

    ws = create_connection(self._ws_uri)

to this

    this.ws = create_connection(self._ws_uri)

with corresponding changes wherever ws is used.

Another issue: the driver does not take advantage of the new WeeWX V4 logging. Fixing this is simple. Change this

These are some handy syslog functions. def logmsg(level, msg):

syslog.syslog(level, 'tempestAPI: %s' % msg)

def logdbg(msg): logmsg(syslog.LOG_DEBUG, msg) def loginf(msg): logmsg(syslog.LOG_INFO, msg) def logerr(msg): logmsg(syslog.LOG_ERR, msg)

to this:

try:

Test for new-style weewx logging by trying to import weeutil.logger

import weeutil.logger
import logging
log = logging.getLogger(__name__)

def logdbg(msg):
    log.debug(msg)

def loginf(msg):
    log.info(msg)

def logerr(msg):
    log.error(msg)

except ImportError:

Old-style weewx logging

import syslog

def logmsg(level, msg):
    syslog.syslog(level, 'tempestAPI: %s:' % msg)

def logdbg(msg):
    logmsg(syslog.LOG_DEBUG, msg)

def loginf(msg):
    logmsg(syslog.LOG_INFO, msg)

def logerr(msg):
    logmsg(syslog.LOG_ERR, msg)

— Reply to this email directly, view it on GitHub https://github.com/livysdad27/tempestWS/issues/1, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABNPU3CZEVOBEKIR4VOZ5SLVVVTJHANCNFSM54P4DXPA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

livysdad27 commented 2 years ago

I created the closeport_logging branch to address this @tkeffer . I'm running it on my server to see how it goes for a few days prior to merging it in. Occasionally the weatherflow websocket server dies off and I want to see if the updated code deals with that more gracefuly.

livysdad27 commented 2 years ago

@tkeffer Thanks again! I ran this branch in production for > 24 hours with no issues. I think there is still room for me to improve checking for connection, attempting to retry if the connection fails etc... but right now this version is cleaner, uses the new logging and doesn't die on close. I also moved the opening of the connection to the main body of the object. Now getLoopPackets really just sits and listens for messages.

Appreciate the help!