paulscherrerinstitute / pcaspy

Portable Channel Access Server in Python
BSD 3-Clause "New" or "Revised" License
32 stars 24 forks source link

Wrong output from Example 2 in Tutorials #94

Open wigging opened 9 months ago

wigging commented 9 months ago

I went through Example 2 in the Tutorials section of the documentation. My code for the example is shown below. I'm trying to run the code using Python 3.10 on macOS Sonoma 14.1.

import _thread
import subprocess
import shlex
from pcaspy import Driver, SimpleServer

class MyDriver(Driver):

    def __init__(self):
        Driver.__init__(self)
        self.tid = None  # Shell execution thread id

    def write(self, reason, value):
        status = True
        if reason == 'COMMAND':
            if not self.tid:
                command = value
                self.tid = _thread.start_new_thread(self.runShell, (command,))
            else:
                status = False
        else:
            status = False

        # Store the values
        if status:
            self.setParam(reason, value)

        return status

    def runShell(self, command):
        # Set status BUSY
        self.setParam('STATUS', 1)
        self.updatePVs()

        # Run shell
        try:
            proc = subprocess.Popen(shlex.split(command), stdout=subprocess.PIPE, stderr=subprocess.PIPE)
            proc.wait()
        except OSError as m:
            self.setParam('ERROR', str(m))
            self.setParam('OUTPUT', '')
        else:
            self.setParam('ERROR', proc.stderr.read().rstrip())
            self.setParam('OUTPUT', proc.stdout.read().rstrip())

        # Set status to DONE
        self.setParam('STATUS', 0)
        self.updatePVs()
        self.tid = None

def main():

    prefix = 'MTEST:'
    pvdb = {
        'COMMAND': {
            'type': 'string',
        },
        'OUTPUT': {
            'type': 'string',
        },
        'STATUS': {
            'type': 'enum',
            'enums': ['DONE', 'BUSY']
        },
        'ERROR': {
            'type': 'string',
        },
    }

    server = SimpleServer()
    server.createPV(prefix, pvdb)
    MyDriver()

    while True:
        # process CA transactions
        server.process(0.1)

if __name__ == '__main__':
    main()

I get the following when I run caput:

❯ caput MTEST:COMMAND "whoami"
Old : MTEST:COMMAND                  
New : MTEST:COMMAND                  whoami

Next, I get what is shown below after running caget:

❯ caget MTEST:OUTPUT
MTEST:OUTPUT                   103

The output is wrong and it should give my user id for the computer which is gavinw not 103.

xiaoqiangwang commented 9 months ago

pysh.py has switched to use char type for COMMAND and OUTPUT early on, so that the tutorial code has not been tested after supporting Python 3. The basic problem is that string typed PV only accepts str input not bytes. Thanks for spotting this broken code.

wigging commented 9 months ago

@xiaoqiangwang So you're saying that I should use this pysh.py script for Example 2 in the Tutorials?

xiaoqiangwang commented 9 months ago

Try it. It requires adding -S switch to caget and caput programs.

$ caput -S MTEST:COMMAND whoami
Old : MTEST:COMMAND 
New : MTEST:COMMAND whoami
$ caget -S MTEST:OUTPUT        
wigging commented 9 months ago

I went through the pcaspy/example/pysh.py script in this repository and came up with a modified version of the example (see below).

import threading
import subprocess
from pcaspy import Driver, SimpleServer

class MyDriver(Driver):

    def __init__(self):
        Driver.__init__(self)
        self.th = None

    def write(self, reason, value):
        status = True

        # Take proper actions when reason is COMMAND
        if reason == "COMMAND":
            if not self.th:
                command = value
                self.th = threading.Thread(target=self.runShell, args=(command,))
                self.th.start()
            else:
                status = False
        else:
            status = False

        # Store the PV values
        if status:
            self.setParam(reason, value)

    def runShell(self, command):
        print("DEBUG: Run ", command)

        # Set status to BUSY
        self.setParam("STATUS", 1)
        self.updatePVs()

        # Run shell command
        try:
            proc = subprocess.run(
                command.split(), capture_output=True, check=True, text=True
            )
            self.setParam("ERROR", proc.stderr.rstrip())
            self.setParam("OUTPUT", proc.stdout.rstrip())
        except subprocess.CalledProcessError as e:
            self.setParam("ERROR", e.stderr)
            self.setParam("OUTPUT", "")

        # Notify that processing is done
        self.callbackPV("COMMAND")

        # Set status to DONE and reset thread attribute
        self.setParam("STATUS", 0)
        self.updatePVs()
        self.th = None

        print("DEBUG: Finish ", command)

def main():
    prefix = "MTEST:"

    pvdb = {
        "COMMAND": {"type": "string", "asyn": True},
        "OUTPUT": {"type": "string"},
        "STATUS": {"type": "enum", "enums": ["DONE", "BUSY"]},
        "ERROR": {"type": "string"},
    }

    server = SimpleServer()
    server.createPV(prefix, pvdb)
    MyDriver()

    while True:
        server.process(0.1)  # Process CA transactions

if __name__ == "__main__":
    main()

This allows me to run the following without having to provide the -S option:

$ caput MTEST:COMMAND whoami
Old : MTEST:COMMAND
New : MTEST:COMMAND whoami
$ caget MTEST:OUTPUT
MTEST:OUTPUT gavinw

It works fine on my computer and I get the expected result from the caget output.

xiaoqiangwang commented 9 months ago

Yes, the text option will return str instead of bytes. But the tutorial and also the example programs were written to be compatible with Python 2 and 3. It would be best to support bytes input for string typed PVs.

wigging commented 9 months ago

Python 2 reached end of life almost 4 years ago (see here). You should not support Python 2.