fake-name / PyIOTech

Python wrapper for IOTech/Measurement-Computing data-acquisition devices
GNU General Public License v2.0
2 stars 3 forks source link

Remove code duplication #2

Closed u55 closed 8 years ago

u55 commented 8 years ago

There is a considerable amount of code duplication between the files in the "/examples/" directory and the "/examples/nextGen/" directory. By the name, it seems that the files in the "nextGen" directory were intended to replace the other files. Can we remove whichever duplicated files are no longer needed and clean up the "/examples/" directory?

fake-name commented 8 years ago

The "nextGen" folder was intended to be a complete rewrite of how the UI worked, and to eventually replace the "/examples/" dir entirely. That's why it got duplicated.


Basically, the IOTech has a /LOT/ of configuration options, and writing a UI for managing them procedurally would be kind of a nightmare. The idea was to have a declarative mini-language that encapsulated the concept of an editable "value", that could then generate the appropriate configuration struct for the IOTech.

I actually wound up implementing this architecture recently for a different project, at my current job, but it never got completed here.

Basically, feel free to delete one or the other as you want. You should be added as a collaborator at this point.

u55 commented 8 years ago

Well, if it were entirely up to me, I would remove all of the GUI code and leave only self-contained single-file example programs. But I don't want to pressure you into removing anything that you found useful. I never tried running any of your GUI examples, and I have no idea if they work or how to modify them to my use case.

If anyone else besides myself uses this library, they are probably going to want to write their own GUI application using their favorite framework (tkinter, pyqt/pyside, wxpython, etc.) and they only want to know the minimum information necessary from this library in order to interact with the device driver. That's exactly what I did for my use case.

It seems to me that a declarative mini-language would only be useful to other people if it completely implements 100% of the device functionality and if it is extremely well documented.

u55 commented 8 years ago

@fake-name Let me also take the time to thank you for writing this python wrapper. It saved me from having to modify and maintain an old LabVIEW program that someone else wrote years ago. I would much rather write something from scratch in python than spend a minute of my life working with buggy and annoying LabVIEW!

fake-name commented 8 years ago

@u55 - The UI implements a fairly easy way to monitor and log the data streams. Basically, it does realtime plotting and logging. For initial validation, and general testing, we (myself and the collaborators I wrote this for) found it tremendously valueable. It basically turns the IOTech device into an oscilloscope with infinite sample depth.

The declarative mini-language was to implement a UI-based control surface that would indeed expose the complete device functionality. The end goal there was to have a python-based application that could be used without having to know python, do log data from IOTech devices. Having a custom language for something like a config file is somewhat silly, it's easier to just use a python object (which is how the config currently works, if you're using any of the whole tools I wrote).

I suspect that most people (or at least myself) are far happier starting with a functional UI base, and just hacking on it to do what they want then starting from scratch. I know that for the first few years of my experience with UI things, that's exactly what I did.


Basically, the goal in /nextgen/ was a tool a non-programmer could use and possibly tweak for their own purposes. It was supposed to not require any modification to expose all the hardware's functions. It was also never really finished, and then I changed jobs.


Also, it should be noted that this isn't entirely my own work. It's based on pydaqboard, which I updated for more recent hardware, and spent a while futzing with to make it work again.

On that note, the repo really needs a license note. It's apparently GPLv2, as that's what pydaqboard is.

u55 commented 8 years ago

Cool. Then can I suggest that we give the UI application a name, like "DaqGUI", and simplify the examples directory to something like the following:

examples/
   ├── DAQAdcRd.py
   ├── DaqDacWt.py
   ├── DaqTimer.py
   ├── DaqGUI/
   │   ├── dynamic_image_wxagg.py
   │   ├── GraphPanel.py
   │   ├── GUI.py
   │   ├── main.py
   │   ├── pyDaqBoard/
   │   │   ├── DAQScan.py
   │   │   ├── __init__.py
   │   │   └── ringBuffer.py
   │   ├── queVars.py
   │   ├── settingsWindow.py
   │   └── test.py

The three programs "DAQAdcRd.py", "DaqDacWt.py", and "DaqTimer.py" really are independent self-contained programs and should be separate from the GUI application, in my opinion. I also have another self-contained example program that I can add.

fake-name commented 8 years ago

looks good to me

u55 commented 8 years ago

The two files "dynamic_image_wxagg.py" and "pyDaqBoard/ringBuffer.py" are not used and they look ancient. Is there any reason to keep them?

fake-name commented 8 years ago

"pyDaqBoard/ringBuffer.py" appears to have migrated into queueVars.py, so I'd say it can be safely removed.

There's two instances of it too.

I think "dynamic_image_wxagg.py" is what I used as reference when writing graphPanel.py. It can probably also be deleted. This was, I believe, one of the first projects where I dealt with doing my own drawing on a DC. It's probably non-ideal.

A lot of this is from when I was just getting started with version control, and it's rather messier then it should be.

u55 commented 8 years ago

Added license information with commit 6fbf166d658c6fbff65acbe6edc253b254ac014b.

fake-name commented 8 years ago

Let me know if you have more questions about stuff. I'm pretty sure there's a non-zero idiot-component coefficient in the codebase. It's been a while since I looked at any of this stuff, and it's nice that someone else is benefiting from it, and I'm happy to help try to figure out why I did the dumb stuff I did.

u55 commented 8 years ago

@fake-name Great. Thanks.