1

I'm using this forum for some time, but this is the first time that I ask a question, since I haven't been able to find a good way around my difficulties, and because I hope this question will be useful to other people too.

I am implementing a simple notification board, i.e. a window where messages coming from a socket connection are displayed. This board prints in red the last message received and in blue the old ones, up to ten. When a message sent by the client is 'Q', the connection terminates and the notification board is destroyed.

I am using Tkinter, threading and sockets, but the behaviour is not smooth (it takes a while to refresh the notification board). I can think of a few problems: the thread handling the connection is not closed; the update of the window is performed by destroying and recreating the toplevel. Unfortunately I can't understand if these issues are the source of the problem.

Here is the code for the client, a very simple one:

#!/usr/bin/env python

import socket

HOST = ''           # Symbolic name meaning the local host
PORT = 24073        # Arbitrary non-privileged port

s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
s.connect((HOST,PORT))

while True:
    message = raw_input('Enter your command (Q=quit): ')
    s.send(message)
    reply = s.recv(1024)
    if reply=='Q':
        print 'Request to disconnect was received.'
        break
    else :
        print reply
s.close()

And here is the server. The server has implemented a class that handles the notification board characteristics, a thread for the socket connection and finally the main part with the mainloop().

#!/usr/bin/env python

import socket
import threading

from Tkinter import *
from datetime import datetime

### Class definition

class NoticationsBoard() :

    def __init__(self, title):
        self.messages = []
        self.toplevel = None
        self.title = title
        self.backgroundColor = 'black'
        self.textColor = 'blue'
        self.textColorFirst = 'red'
        self.boardSize = '200x250+0+0'
        self.listsize = 10

    def createBoard(self):
        self.toplevel = Toplevel()
        self.toplevel.title(self.title)
        self.toplevel.configure(background='black')
        self.toplevel.geometry(self.boardSize)

    def publish(self, message):
        self.addToList(message)
        self.displayList()

    def addToList(self, msg):
        if len(self.messages) == self.listsize:
            self.messages.pop(0)
        timestamp = datetime.utcnow().strftime('%H:%M:%S')
        newMessage = (msg, timestamp)
        self.messages.append(newMessage)

    def displayList(self):
        # Destroy and create the window (is it really necessary?)
        if self.toplevel is not None :
            self.toplevel.destroy()
        self.createBoard()
        # create labels for all the messages in the list
        index = 1
        for m, t in self.messages :
            color = self.textColor
            if index == len(self.messages) :
                color = self.textColorFirst
            label = Label(self.toplevel, text=m, height=0, width=100, fg=color, anchor=W)
            label.grid(row=0,column=1)
            label.configure(background=self.backgroundColor)
            label.pack(side='bottom')
            index = index +1

####### Run

def receiveMessages(newsboard) :
    print '===== Inside receiveMessages ======'
    s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
    print 'Socket created'
    try:
        s.bind((HOST, PORT))
    except socket.error , msg:
        print 'Bind failed. Error code: ' + str(msg[0]) + 'Error message: ' + msg[1]
        sys.exit()
    print 'Socket bind complete'
    s.listen(1)
    print 'Socket now listening on port', PORT
    # Accept the connection once
    (conn, addr) = s.accept()
    print 'Connected with ' + addr[0] + ':' + str(addr[1])
    stored_data = ''
    while True:
        # RECEIVE DATA
        data = conn.recv(1024)
        # PROCESS DATA
        if data == 'Q' :
            print 'Client wants to disconnect.'
            reply = 'Q'
            conn.send(reply)
            break
        else :
            print data
            newsboard.publish(data)
            reply = 'Message received:' + data
            conn.send(reply)
    print 'Close connection.'
    conn.close()
    board.destroy()

HOST = ''   # Symbolic name meaning the local host
PORT = 24073    # Arbitrary non-privileged port

app = Tk()
app.title("GUI main")

board = NoticationsBoard('Notifications')    

t = threading.Thread(target=receiveMessages, args = (board,))
t.start()

app.update()    # Not sure what it does and if it is necessary
app.mainloop()

I am using Python 2.7.5.

Finally, but this is something minor, I was trying to display the timestamp of each message on the left of the message itself, in a different color. It seems to me that it is not possible to have text of different colours on the same label, so I had created other labels in the for loop with the timestamps. I tried to display the timestamp and message labels one next to the others using .grid(column=0) and .grid(column=1), but they were not one next to the other but one below the other, and I haven't figured out why.

As you understood, I am not a skilled programmer, and definitely a newbie with Python...

Thanks in advance to whom will give me some advice, and I hope this question will be useful to many people.

user1472709
  • 133
  • 2
  • 14
  • Possibly a duplicate of http://stackoverflow.com/q/16745507/291641. Calling UI methods from a different thread is likely to cause problems. As suggested, use a queue and poll the queue from the UI thread should solve the issues. Or use Twisted to make the whole run with 1 thread in an event oriented manner. – patthoyts Mar 17 '14 at 00:13
  • Thank you patthoyts for your suggestions. I am not sure to have understood how I can avoid calling UI methods from the threaded `receiveMessages()` function. I looked at the question you linked but I struggle to see where the `queue` and the `poll` would need to be added. As for Twisted, an event oriented tool could be a good idea but I am wondering if that would be excessive given the simplicity of the task that theoretically this code should perform. Would an event driven implementation simplify the whole structure in your opinion? – user1472709 Mar 17 '14 at 00:33
  • Quick update: I am rewriting the code starting from the recipe written by Jacob Hallén (http://code.activestate.com/recipes/82965-threads-tkinter-and-asynchronous-io/). As soon as I get it working, I will upload my working solution (which probably won't be the best possible, but yet!). – user1472709 Mar 19 '14 at 11:28

1 Answers1

0

Ok, it seems like I found a solution by taking parts of other people's questions, suggestions, and code. There are few differences in the appearance maybe. In the GUI, the most notable one is that I preload all the Labels and then I just modify the text. In the threading part, well, that's completely changed. Please see below.

#!/usr/local/bin/python

try:
    import Tkinter
except ImportError:
    import tkinter as Tkinter
import time
import threading
import random
import Queue
import socket
import sys
from datetime import datetime

class GuiPart:
    def __init__(self, master, queue):
        self.queue = queue
        # GUI stuff
        self.labelArray = []
        self.messages = []
        # Set up the GUI
        self.master = master
        self.backgroundColor = 'black'
        self.listsize = 10
        master.config(bg=self.backgroundColor)
        self.board = Tkinter.LabelFrame(self.master, text='Notification Board',     bg='Black', fg='Yellow', labelanchor='n', width=170)
        self.initLabels()
        self.board.pack()

    def initLabels(self) :
        self.textColorTime = 'cyan'
        self.textColorMessage = 'orange'
        colorTime = 'blue'
        colorMessage = 'red'
        for i in range(0,self.listsize):
            la = Tkinter.Label(self.board, height=0, width=10, bg=self.backgroundColor, fg=colorTime, anchor=Tkinter.W)
            lb = Tkinter.Label(self.board, height=0, width=160, bg=self.backgroundColor, fg=colorMessage)
            la.grid(row=i,column=0, sticky=Tkinter.W)
            lb.grid(row=i,column=1, sticky=Tkinter.W)
            self.labelArray.append((la, lb))
            colorTime = self.textColorTime
            colorMessage = self.textColorMessage
            self.initList()
        self.displayList()

    def initList(self):
        for i in range(0, self.listsize):
            t = ''
            m = ''
            self.messages.append((t,m))

    def processIncoming(self):
        while self.queue.qsize():
            try:
                msg = self.queue.get(0)
                self.processMessage(msg)
            except Queue.Empty:
                pass

    def processMessage(self, message):
        timestamp = datetime.utcnow().strftime('%H:%M:%S')
        self.publish(timestamp, message)

    def publish(self, msg1, msg2):
        self.addToList(msg1, msg2)
        self.displayList()

    def addToList(self, msg1, msg2):
        if len(self.messages) == self.listsize:
            self.messages.pop(0)
        if (msg1 == None):
            msg1 = datetime.utcnow().strftime('%H:%M:%S')
        newMessage = (msg1, msg2)
        self.messages.append(newMessage)

    def displayList(self):
        index = self.listsize -1
        for t, m in self.messages :
            la, lb = self.labelArray[index]
            la.config(text=t)
            lb.config(text=m)
            index = index -1

    def destroy(self):
        self.master.destroy()

class ThreadedClient:

    def __init__(self, master):
        self.master = master
        # Create the queue
        self.queue = Queue.Queue()
        # Define connection parameters
        self.conn = None
        self.bindError = False
        # Set up the GUI part
        self.gui = GuiPart(master, self.queue)
        # Set up the thread to do asynchronous I/O
        self.running = True
        self.commThread = threading.Thread(target=self.workerThreadReceive)
        self.commThread.daemon = True
        self.commThread.start()
        # Start the periodic call in the GUI to check if the queue contains anything
        self.periodicCall()

    def periodicCall(self):
        if not self.running:
            self.killApplication()
        else :
            self.gui.processIncoming()
            self.master.after(100, self.periodicCall)

    def workerThreadReceive(self):
        s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
        try :
            s.bind((HOST, PORT))
        except socket.error as msg :
            print 'Bind failed. Error code: ' + str(msg[0]) + ' Error message: ' + str(msg[1])
            self.running = False
            self.bindError = True
            return
        s.listen(1)
        (self.conn, self.addr) = s.accept()
        while self.running :
            data = self.conn.recv(1024)
            if data == 'Q' :
                self.conn.sendall('Q')
                self.running = False
            else :
                self.queue.put(data)
                reply = 'ACK'
                self.conn.sendall(reply)
        if self.conn is not None:
            self.conn.close()

    def killApplication(self):
        self.running = False
        if (self.conn is None) and (not self.bindError) :
            sfake = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
            sfake.connect((HOST,PORT))
            sfake.sendall('Q')
            sfake.close()
        self.gui.destroy()
        sys.exit()


HOST = ''       # Symbolic name meaning the local host
PORT = 24073    # Arbitrary non-privileged port

root = Tkinter.Tk()

client = ThreadedClient(root)

root.protocol("WM_DELETE_WINDOW", client.killApplication)
root.mainloop()

The client is the same as in the question.

I am not sure my code is the most elegant one (ok, let's say it... it is not!), but it seems to do the job. Nevertheless, I would like to get your feedback about it, because I am sure that I have overlooked many issues and that things could have done in an easier way. :)

user1472709
  • 133
  • 2
  • 14