[LinuxCNC/linuxcnc PR#274] Permit to insert a gladevcp default interface using stepconf

#1 – jepler 于 2017-07-27

Hi @nicokid.

I apologize that this pull request has gone so long without action on our part, because it clearly represents a lot of effort on your part.

Is this pull request in a condition where you believe it is suitable to merge (i.e., no new bugs you know about)? Can you update the title of this pull request, and make sure the top comment describes what is new in stepconf? (You should be able to do this by the “edit” button to the right of the title and the “pencil” button above the top comment)

In the future, one thing that would be helpful is to make sure that each pull request is “relatively small”. One aspect of how github works, which you may not be aware of, is that when you make a pull request for some branch (e.g., your master branch), any commits you add to it subsequently go in that same original pull request. It is better to make sure that 1 branch = 1 bugfix or feature = 1 pull request, so that I don’t have to review, test, and approve 69 commits all at one time.

#2 – nicokid 于 2017-07-27

> Is this pull request in a condition where you believe it is suitable to merge (i.e., no new bugs you know about)?

At this time I think there are many bugs. I have to fix some problems with gladevcp. Please wait some days.

> Can you update the title of this pull request, and make sure the top comment describes what is new in stepconf? (You should be able to do this by the “edit” button to the right of the title and the “pencil” button above the top comment)

Ok, as soon as I fix the code I correct the title and comments.

> In the future, one thing that would be helpful is to make sure that each pull request is “relatively small”. One aspect of how github works, which you may not be aware of, is that when you make a pull request for some branch (e.g., your master branch), any commits you add to it subsequently go in that same original pull request. It is better to make sure that 1 branch = 1 bugfix or feature = 1 pull request, so that I don’t have to review, test, and approve 69 commits all at one time.

Eh, in fact I realized that the subsequent changes end up in the “pull requests”
Unfortunately i use github as my harddisk backup (like my subversion) and then in the pull request i got stuff with bugs. Sorry, I will try to be better in the future.

#3 – jepler 于 2017-07-27

@nicokid Thanks for your response

It makes perfect sense to push things to github even when they don’t work 100% yet.

I’ll wait to hear from you again about this pull request. I retitled the pull request to reflect that it is a work in progress, and with my impression of one of the most important goals of your work.

#4 – nicokid 于 2017-08-24

Hi Jeff,
I did a little test and the program seems to be running.
For the graphic interface part in pyvcp and gladevcp i miss something for the spindle speed because i do not have an hardware available. I was limited to leaving what was before.

Nicola.

#5 – nicokid 于 2017-08-27

Sorry for this new commit Jeff, I noticed a problem.

Nicola.

#6 – nicokid 于 2017-09-12

Hi Jeff,
did you make any checks here?

Nicola.

#7 – c-morley 于 2017-10-16

I’m curious why you broke the pages.py code into a many different pages… I find following the code now difficult.
For instance you now have a file named axis.py which i believe hold all the helper function for axis data collecting (pulled out of stepconf.py i bet) -might be better called axishelperfunctions rather then something so close to all the other axis python files.

pages.py was purposely made one file to make it easy to debug and follow the code.
While i could see possible merit in breaking stepconf into a couple files – I certainly don’t find looking through 26 files easier then 5

I remember when I first started with linuxcnc on the classicladder update project – that following the code (classicladder uses a similar breakup of files that you used) required having many many files open at once – not very convenient.

My 2 cents worth – looks like you’ve added lots of interesting stuff in there.

Chris M

#8 – nicokid 于 2017-10-17

Happy to hear you Chris.
The reason why I split the page.py file is that now every glade file has its own python file with its own functions. This makes it easier to debug the single “tab” of interface because the functions are all there. For example, for the finished.glade file exists a finished.py file. Probably I’m too little pythonic and too much tied to c language

Regarding the axis.py file I totally agree with you, the right name should be axishelperfunctions. I’ll change it, thank you.

This is the list of python files with what they are doing:
— files with instructions for a .glade file —
main_page.py
start.py
base.py
pport1.py
pport2.py
axisa.py
axisu.py
axisv.py
axisx.py
axisy.py
axisz.py
spindle.py
options.py
halui_page.py
gui_page.py
finished.py

— and helper for axis —
axis.py (axishelperfunctions.py)

— files that were already present —
stepconf.py
pages.py
import_mach.py
build_HAL.py
build_INI.py

— files that I added —
definitions.py (like c I prefer to get all constants in only one file)
preset.py (here we can add all parameters for presetting cnc machines)
gui_gladevcp.py (construction of the gladevcp interface)
gui_pyvcp.py (construction of the pyvcp interface)
tool_change.py (function for tool change, very beta release…)

In the end the last 4 files is what I added really as new. The guigladevcp.py and guipyvcp.py files are so ugly to see that I really do not want to put them in stepconf.py!!! This is because the inside printf functions are too long.

I bought a “stepcraft 840” cnc with parallel interface to build a panel for my flight simulator. Being a linux user I tried to use linuxcnc. Unfortunately, stepconf is just for the initial configuration and the rest should be done by hand. I hate these things…
To add the features I need, I had to lose a ton of hours learning how stepconf works. And the radio panel for my simulator is not yet built

I’m sorry if the changes made are not pretty good, I’m not a really good programmer.
Nicola.

#9 – c-morley 于 2017-10-17

your programming looks fine – I’m not a professional programmer by any stretch – self taught mostly on this project.
I do understand the method to your madness (a py and glade file for each tab) – I just completely disagree about debugging. I am looking at debugging from the position of someone not knowing the code already.
It is always easier to debug your own code especially when it is new in your mind.

For instance by breaking everything into many files make search, search and replace painful.
Having to have many files open to go through the flow of the code is inconvenient.
Having files with names that don’t really give much clue as to their code use makes understanding take longer.
eg Stepconf.py what is it’s purpose ?
– It has environment setup code,
– it has some machine data defaults
– it has private data (which there is now only two pieces?) but there is also a file somehere else with other data
– it has GUI axis? helper code

It would make more sense to me to have stepconf set up the ervironment and probably have the code that operates the pages (pages.py) in it and put the data (all data definitions and machine data) in one file called data.py with two classes : machine data and private data, private data having the definitions in it.

Pages.py
The comments on the very top are out of date.
Importing methods from other files is a very smart but unusual procedure – I would add comments to explain.

When I we factored Jeff original code, I too decided that stepconf was too big and breaking out code would be helpful, looking back now I think I should have pulled the pages helper code out but i would have added them all to one file.

I feel like I’m beating you up and that is not my intention.
A lot of this is personal preference I suppose. I’ve been in the group for many years and one tends to get opinionated after having to follow and pick up on other people’s code (or even my own after a couple years). I have learned a lot about maintainable code from the project and it’s people.

Of course it seems you are the maintainer of stepconf now, so you can do what works for you, I just couldn’t keep my mouth shut on my ideas

Chris M

#10 – nicokid 于 2017-10-18

> I do understand the method to your madness (a py and glade file for each tab) – I just completely disagree about debugging. I am looking at debugging from the position of someone not knowing the code already.
>
Ok, at the moment for my programmer skills I’m getting easier to have more files at least for the glade and python mix. Maybe my fault is that I use too many time “grep” for my research and so it does not matter if there are many files.

> Having files with names that don’t really give much clue as to their code use makes understanding take longer.
>
Yes, you are right. I admit I have little fantasy in creating file names, every suggestion is well accepted.

stepconf.py and pages.py
I completely agree with you about your comments on these two files. I have simply emptied them gradually because of my madness for splitting files They are currently being cleaned.
Ok for putting pages.py into stepconf.py and ok for creating data.py.

code comments
It is true that my code has a few comments and I apologize for that. My English is so bad.

Meanwhile, thanks for your comments. It is quite clear that here you and Jeff are the best 👍.
I have very little experience on linuxcnc and I really appreciate your comments. And don’t worry, it’s impossible to hurt me

Nicola.

#11 – nicokid 于 2017-10-24

Jeff added the require_version control at the beginning of stepconf.py. It’s right.
But probably in the last few months I’ve changed something to the program and now it’s hard to align without any further changes. Sorry.
Nicola.