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

Wrong output from Example 2 in Tutorials #94

Open
wigging opened this issue Nov 29, 2023 · 6 comments
Open

Wrong output from Example 2 in Tutorials #94

wigging opened this issue Nov 29, 2023 · 6 comments

Comments

@wigging
Copy link

wigging commented Nov 29, 2023

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
Copy link
Collaborator

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
Copy link
Author

wigging commented Nov 29, 2023

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

@xiaoqiangwang
Copy link
Collaborator

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
Copy link
Author

wigging commented Nov 29, 2023

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
Copy link
Collaborator

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
Copy link
Author

wigging commented Nov 30, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants