Re-factoring shell scripts in python for Phoniebox 2.x #1421
Conversation
scripts/PlayoutControl_CLI.py
Outdated
| elif(args_main['command'] == "playout_position_save"): | ||
| playProcess.playout_position_save() | ||
|
|
||
| elif(args_main['command'] == "playout_resume_play"): |
There was a problem hiding this comment.
Also, have you considered to make a module with functions and call them by their name? This might allow you to have this in separated files (or at least individual (testable) methods).
https://stackoverflow.com/questions/3061/calling-a-function-of-a-module-by-using-its-name-a-string
@arne123 does the same on future3/develop in the RPC Client
There was a problem hiding this comment.
@pabera : If I understand you correctly, your suggestion would shorten most the script into more or less one line like on future3/develop in the RPC Client
if (call_function is not None): # better to check with is callable() ??
If I interpret the typo in the comment of this line correctly as "check which", then this is what I would like to do in the CLI file.
Check for valid commands.
And if no valid command is given, possibly give an explanation what is possible and what not.
I know it's extra work, but because I will need to change the scripts called from shell to python in so many places, I would like to have that abstracted error check. Does that make sense? Or did I understand you wrong?
There was a problem hiding this comment.
You are right. at least the repetivie code like this would shrink into 1 line.
elif(args_main['command'] == "playout_position_save"):
playProcess.playout_position_save()
| if('CMDSHUFFLE' in conf): | ||
| if(args_main['cardid'] == conf['CMDSHUFFLE']): |
There was a problem hiding this comment.
Here and below.. how about writing it into a combined condition? And it seams that Python does not necessarily require ( ) around the condition statement
if 'CMDSHUFFLE' in conf and args_main['cardid'] == conf['CMDSHUFFLE']:
There was a problem hiding this comment.
I wanted to be really nice... because if I combine both, the second condition throws an error.
There was a problem hiding this comment.
I might be mistaken, but Python should not do this.. if you have
if A and B and A = False, it shouldn't matter what B is. But I don't have much experience with Python, so someone else might speak about it.
@MiczFlor Could you try to add |
scripts/PlayoutControl_CLI.py
Outdated
| playProcess.sys_switches(args_func) | ||
| return | ||
|
|
||
| elif(args_main['command'] == "playout_position_save"): |
There was a problem hiding this comment.
I would still write if instead of elif
| def read_config_bash(paths_all): | ||
|
|
||
| # paths_all must be a list like [a, b, c] | ||
| conf = {} | ||
| for path_config in paths_all: | ||
| # read config | ||
| with open(path_config) as myfile: | ||
| for line in myfile: | ||
| if not line.lstrip().startswith('#'): | ||
| name, var = line.partition("=")[::2] | ||
| conf[name.strip()] = var.strip() | ||
| # strip " off values in dictionary conf | ||
| conf = {k: v.strip('"') for (k, v) in conf.items()} | ||
| return conf |
There was a problem hiding this comment.
I would add a try / catch to catch issues in situations where the file does not exist
|
@MiczFlor, with future3 under dev is this still needed? |
|
Closing this now as future3 is an all Python implementation |
Refactoring the shell scripts in python for version 2.x