LazeMSS / OctoPrint-TopTemp

Topbar temperature plugin for OctoPrint
21 stars 2 forks source link

[Feature Request] custom execute execution in bash #95

Closed puterboy closed 1 year ago

puterboy commented 1 year ago

It would be helpful/convenient if custom commands were executed in the Bash rather than Bourne shell. For example, 'echo' in the Bourne shell doesn't recognize the '-e' flag to allow for control characters like linefeeds. This caused (unexpected) errors for me. Of course it can be fixed by starting the command with "bash -c" but that is a bit messy.

Also at least in my default RPI setup, running Python and executing a shell command uses the Bash shell so the difference is unexpected.

LazeMSS commented 1 year ago

Added in: https://github.com/LazeMSS/OctoPrint-TopTemp/releases/tag/0.0.2.3

puterboy commented 1 year ago

Your fix seems to invoke 'bash' when running the command under 'test' but it doesn't seem to be running under bash when the actual command is run by TopTemp every N seconds.

Specifically:

  1. If I manually wrap the command in 'bash -c' then it works both from 'test' and in the regular update loop
  2. If I don't manually wrap the command in 'bash -c', then it works from 'test' (and didn't before your update) but it fails to run properly in the regular update loop.

I imagine that this should be a simple fix...

LazeMSS commented 1 year ago

I would think it does - it runs using https://github.com/LazeMSS/OctoPrint-TopTemp/blob/7f5fc618b95fdf0facc25b0684c8f6bc7e7666e2/octoprint_toptemp/__init__.py#L574 which again calls https://github.com/LazeMSS/OctoPrint-TopTemp/blob/7f5fc618b95fdf0facc25b0684c8f6bc7e7666e2/octoprint_toptemp/__init__.py#L876

The test and the timer uses the same.

puterboy commented 1 year ago

Strange... I am having difficulty tracking down the error. One interesting thing is that the timer version seems to run once correctly, then it fails to update subsequently on subsequent loops, leaving the initial value frozen in the navbar and the pop-up graph blank. Of course, if I manually include 'bash -c' all works fine...

I just can't figure out why the 'Test' version works while the timer loop version doesn't given that they both seem to call the same runcommand function like you mentioned above.

puterboy commented 1 year ago

And strange because now it seems to be working... The only change I made was converting double quotes (") to single quotes ('). Perhaps there was some strange shell substitution going on that only affected the saved timer version and not the test command version?

I will consider this fixed for now :)