motor-expert-screen error messages with NoMachine

[silkenelson]

Expected Behavior

I suspect that this does not work when not running locally (or at least not through NoMachine). There should be some test to only move the motor when this is a 'direct' session if possible. I have not tried to use 'plain' ssh & x forwarding.

Current Behavior

When running motor-expert-screen via NoMachine, I get an error message regarding some failed test in line 141. This is where xdotool is used to get the screen position and we try to move the screen to where the mouse is.

Possible Solution

Steps to Reproduce (for bugs)

Context

Your Environment

[silkenelson]

Oh well, clicked create too soon.
when running motor-expert-screen via NoMachine, I get an error message regarding some failed test in line 141. This is where xdotool is used to get the screen position and we try to move the screen to where the mouse is. I suspect that this does not work when not running locally (or at least not through NoMachine).
There should be some test to only move the motor when this is a 'direct' session if possible. I have not tried to use 'plain' ssh & x forwarding.

[ZryletTC]

I can't seem to reproduce this behavior. Can you describe your NoMachine settings or show me in person?

[silkenelson]

I see this problem when I open a second motor window as xdotool now returns data of a different format and the script chokes (my suspicion, not confirmed).

[ZryletTC]

Ok, I am able to reproduce it now and it's not limited to NoMachine, I also get these errors through a regular x-forwarding ssh session:

/reg/neh/home/pennebak/bin/motor-expert-screen: line 141: [: : integer expression expected
/reg/neh/home/pennebak/bin/motor-expert-screen: line 141: +   : syntax error: operand expected (error token is "+   ")

I'll take a stab at finding a fix for it.

[ZryletTC]

It looks like the issue is that the window often takes longer than the 2 seconds allotted for it to load.
I was able to get rid of the errors by simply changing the sleep 2 to some larger number:
https://github.com/slaclab/engineering_tools/blame/master/scripts/motor-expert-screen#L104

I was able to get away with using sleep 8 but it might be better to use 10 seconds just to be safe.
Ideally, we'd move away from this crude way of waiting and actually find out when it's done loading but that'd take a little more work.

[silkenelson]

I don't think we can sleep longer. Even now, the window is visible before it gets moved: I regularly start interacting with a window before xdotool does its job. But it does help when the window gets otherwise hidden or opens in a window really far away. Maybe we should limit it running when there is a pretty large display available. I suggest further thinking (possibly including living with this) before making the sleep longer.

[KaushikMalapati]

I ran into this issue when working on #223. However, I do not think this is related to how long you sleep for before looking for new windows, since the --sync flag to xdtool search should make it block until it finds the window. https://manpages.ubuntu.com/manpages/trusty/man1/xdotool.1.html
What I have found is I get these kinds of errors

/reg/neh/home/pennebak/bin/motor-expert-screen: line 141: [: : integer expression expected
/reg/neh/home/pennebak/bin/motor-expert-screen: line 141: +   : syntax error: operand expected (error token is "+   ")

whenever I try and open two of the same screen

[kaushikm@tmo-control ~]$ motor-expert-screen XPP:USR:MMS:01
[kaushikm@tmo-control ~]$ motor-expert-screen XPP:USR:MMS:01
/reg/g/pcds/engineering_tools/latest-released/scripts/motor-expert-screen: line 159: [: : integer expression expected
/reg/g/pcds/engineering_tools/latest-released/scripts/motor-expert-screen: line 159: +   : syntax error: operand expected (error token is "+   ")

The second screen causes us to set foundMatch to 1 here

engineering_tools/scripts/motor-expert-screen

Line 130 in 0604133

foundMatch=1

which leads to the id variable not being set, leading to xdotool getwindowgeometry returning nothing, meaning we have xpos and ypos set, but not winx, xwidth, winy, or ywidth. The errors are about trying to perform arithmetic on unset variables. I confirmed this by setting the sleep to sixty seconds and observing that I never got the errors unless I didn't close the previous screen I had opened - I originally thought this was an issue with my pull request, so was opening a lot of screens to try and debug and sometimes I did not close the old ones but would use the same pv when I up-arrowed to rereun motor-expert-screen. I also believe this error only shows up if I do not open the first screen before running the second motor-expert-screen. I guess this is what causes them to have duplicate ids. I have to actively click to open the first screen - if I only preview it by hovering over it in the taskbar the errors still show up when opening the second screen

I think this means we can remove the two second sleep (with further testing) which is nice. I'm not sure how to fix this or if it's worth fixing since it seems like a niche issue or if the NoMachine errors this issue was originally for is for something else (I did all this on moba and got the same errors for what it's worth), but I wanted to at least write it down since I spent some time looking into it when I thought it was caused by my pull request and it might be useful if anyone wants to ever fix this.

I should mention this only happens on computers with xdotool installed, like hutch machines, but not psbuild-rhel7 since it doesn't have xdotool so we presumably exit when we find that xpos is unset.

[KaushikMalapati]

I think I wrote my comment prematurely. First, one of Silke's comments above already mentions this problem happening when you open the second screen. Second, I said the sleep was not needed - that is wrong. If you have a screen open with the same title as the one you want to open, --sync will not do anything because it will see the first screen and continue before the second screen loads, which means the second screen will not get a window id put into newwins, leading to this issue. xdotool search has a --pid argument. Maybe the pid of the edm process can be captured right after it's opened and is searched for instead of the name to guarantee uniqueness.