When attempting to use the GoPiGo control panel, attempting to “save and close” silently fails without saving data.
Testing by exhaustion, (setting “print” statements everywhere), I determined that the issue was with the save_robot_constants method in easygopigo3.py
I did a file search for “easygopygo3” and found this file in three different places. All of these files reference a configuration file in /home/pi/Dexter - gpg_config3.json - which does not exist.
Attempting to create it within the GoPiGo noVNC window, (running Geany), it failed with a “permissions” error, because the “Dexter” folder and subdirectories are owned by “root” and the pi user only has read and execute permissions to that folder. Ergo the control panel doesn’t have permission to save files - and fails silently.
I solved this problem with a recursive “chown” over the Dexter directory, changing the ownership to pi:pi. (I left the di_updates directory structure alone.)
I also ran a recursive “chmod” over the same structure, changing the permissions to “755” because I had “tweaked” the permissions of some files to “777” while troubleshooting.
Questions:
Why isn’t everything within pi’s home directory owned by “pi”.
(I suspect that this has to do with these scripts being idiot-proofed.)
a. If this is the case, wouldn’t it be better to have the “Dexter” (A wholly owned subsidiary of Modular Robotics™!), specific code in a different location than /home/pi, since the assumption is that everything in home/pi is writable by user “pi”
If it is ABSOLUTELY, POSITIVELY, TOTALLY, and COMPLETELY necessary for this material to:
a. Be located within /home/pi
and
b. Be owned by “root”
Then:
c. Shouldn’t there be a specific “configuration” directory somewhere that IS writable by “pi”?
Why are there three different copies of the easygopigo3.py files?
a. Which of these three is actually being used?
b. Are these three different files, in three different locations, being used by different routines? If so, why? IMHO, there should be one, and only one, version of an essential include file to prevent inadvertent issues.
(Note: I used the “find” feature within the file utility which may not tell me that two of them are symlinks to the third. If this is the case, my sincerest apologies.)
Since “chown” over an entire directory structure is a rather drastic change, is there a better, “Dexter Approved” solution - or did someone inadvertently forget to chown these directories to “pi” before release? (See also question #6)
IMHO, errors like this should not fail silently. If you wish, I will experiment with raising an alert dialog informing the user that an error has occurred.
Having made wholesale changes to the Dexter directory’s ownership and permissions:
a. Is this correct? (i.e. Is this the way it should have been “from the factory”?)
b. If not, what permission and ownership should I restore to which directories?
pi@Carl:~/Carl $ ls -al /home/pi/Dexter
total 64
drwxr-xr-x 9 pi pi 4096 Jun 22 2019 .
drwxr-xr-x 38 pi pi 4096 Jan 25 20:47 ..
-rw-rw-rw- 1 pi pi 33 Mar 27 2019 black_line.txt
drwxr-xr-x 9 pi pi 4096 Mar 27 2019 BrickPi3
-rw-rw-rw- 1 pi pi 8 Jun 6 2019 detected_robot.txt
drwxr-xr-x 8 pi pi 4096 Mar 27 2019 DI_Sensors
drwxr-xr-x 10 pi pi 4096 Mar 27 2019 GoPiGo
drwxr-xr-x 10 pi pi 4096 Jun 22 2019 GoPiGo3
drwxr-xr-x 10 pi pi 4096 Mar 27 2019 GrovePi
drwxr-xr-x 8 pi pi 4096 Jun 22 2019 lib
drwxr-xr-x 7 pi pi 4096 Mar 27 2019 PivotPi
-rw-rw-rw- 1 pi pi 36 Mar 27 2019 range_line.txt
-rwxr-xr-x 1 pi pi 4269 Mar 27 2019 update_master.sh
-rw-rw-rw- 1 pi pi 227 Mar 27 2019 Version
-rw-rw-rw- 1 pi pi 29 Mar 27 2019 white_line.txt
“Da Bitch part” as they say in Blazing Saddles, is that the source “easygopigo.py” file - whichever one is being used - is not writable by the “pi” user, so there is no direct way for us to change it, aside from gaining root access and changing these files by hand.
IMHO, this goes WAAAAAY beyond ugly, since the user interface, as supplied, should work. If there are dependencies or requirements, they should be a part of the initial configuration.
In my case, everything in “/home/pi” was owned by “pi” except for “Dexter” and “di_updates” which were owned by root.
Everything, (as far as I saw during a cursory examination), within both “Dexter” and “di_updates” was owned by “root” with either 755 or 644 permissions. (I noticed the “644” permissions while I was chmodding “Dexter” back to 755 since I always run commands like that “verbosely” and I saw some “644’s” get changed to “755”.)
I don’t know why there is, (was), this difference. Maybe I did something different? Maybe because I ran Dexter OS prior to Raspbian for Robots? Maybe you chowned everything a while ago?
My version of /home/pi/Dexter/GoPiGo3/Software/Python/Examples/Control_Panel/control_panel_gui_3.py did not have the calibration stuff in it so I downloaded the latest:
As Mr. T used to say in The “A” Team: I pity the fool, PITY the fool - that effs around with system include files!
The only time you should be messing with system supplied includes like this, is if you find a problem that can’t be solved any other way, you document the changes, and create a “pull” request against the master GitHub repository. Though, I admit that this problem could be solved by changing the target of the save file to “/home/pi” which is writable by the user “pi” with no other changes required. (Two changes needed, config load and config save)
Entirely possible - I was chasing down this exact issue last year some time and may have done that. I ended up giving up because all my existing code imports a myconfig.setParameters() that has my wheel base and dia as constants.
BTW:
While I was chasing this down, I decided to play with the control panel dialog.
Viz.:
I added a confirmation message on successful save, if the “Save” button was selected.
(In the same location) I added an error message if the save was not successful.
a. I really wanted to do a modal dialog if the save didn’t work, but I couldn’t figure that out. I think there’s another include file needed to use the “dlg” methods.
I added a “Save” button between “Exit” and “Save and Exit” that saves the current configuration constants without exiting the control panel.
I added an additional method - onSaveOnly - that does everything “onSave” does except exit.
a. This is actually lousy coding. What I would have preferred to do was to have “onSave” capture which of the two buttons was selected and either exit or not as appropriate.
I added explanatory text to the “Wheel Diameter” and “Distance Between Wheels” selections that indicates what changes cause what effects.
While doing that, I adjusted the spacing after the text to cause the input boxes and buttons to align properly. I’m not sure I did it the correct way because I padded with spaces.
I run multiple python programs that all use the EasyGoPiGo3 class with mutexes. The main one is the battery level monitor that also docks and undocks (needs the distance sensor). The RPI-monitor runs several data access instances every 30 seconds, and the I am usually running some test or developing a new feature like IMU or OpenCV instance, so the mutex keeps everyone playing nice to the I2C sensors. I also have a data store that is protected by a DI custom “named” mutex. That one is not the use_mutex=true semaphore though. Both are platform / global muxexes.
I thought of that but decided not to get too crazy.
UI thoughts:
Add a “cancel” button that would revert back to the last saved values.
Experiment with various text colors to provide visual feedback.
a. Movement button text would turn green when active.
b. Successful save message would be green and unsuccessful saves would be red.
c. Blinker and eye buttons would illuminate when active. (Change text color)
Maybe change the button’s color?
Experiment with the “onButtonDown” and “onDoubleClick” methods, if they exist.
a. Movement buttons would respond to “onButtonDown” by remaining active only while pressed.
b. “onDoubleClick” would lock the button on until double clicked or “Stop” was pressed.
Figure out how to embed text into the body of the dialog to provide brief descriptions of features.
Stretch Goals:
Allow for “Save As” and “Load” saved config files.
a. Default at startup would be the Dexter default file name.
Figure out how to actually USE this hard-won calibration data within the day-to-day programming / use of the 'bot. (i.e. So that 90 degree turns are actually 90 degrees, etc.)
Find out how to embed custom startup/shutdown code in the library files to do things like auto-center the “head” servo.
Provide a “help” button feature that would pop-up a dialog or web page with helpful hints and tips.
I like the idea of both a “save” and “save/exit” button. Otherwise I’d need to have a “dirty flag” that would bring up an “unsaved changes” dialog - and so far, pop-up modal dialogs have frustrated me.
Maybe I should add modal dialogs to my “Stretch Goals” list?
If those two files are owned by root on the Buster experimental image, then it’s a booboo on my part. They should be owned by pi and be writeable from the GUI.
Thanks for reporting it, I’ll add it to our test plans (why wasn’t it there in the first place, I don’t know)
Perhaps you mistakenly tested as “root” instead of “pi”?
I think I’m using Buster. I’ll have to make sure. Even if I’m on the “stable” version and not Buster, shouldn’t this mandate an update?
===================================
Because we’re all human, that’s why. Been there, done that, bought the T-shirt.
Don’t sweat it, you got us watchin’ your back!
(It’s the least I can do to help repay what you and your colleagues have done for me. I am still floored by the “new user of the month” award - wow!)
===========================
What do you think of my “enhancements” to the control panel?
BTW, is there a way to capture the actual button selected within onSave? I’d rather do that than write an entirely separate method that does 99% of what onSave is already doing.
============================
Just had a thought:
Change onSave to remove the call to onClose.
Create a new method onSaveExit which would call both onSave and onClose.
