Refactor and Test #131
Comments
|
yep. all good ideas. If possible, I'd like two things in reference to this...
number 2 may not be easy or even possible, but number 1 we can do for sure. |
|
Is there any work being done on this? I'd certainly like to add support for some new things to this project, but the current layout makes it more difficult than necessary. It appears that breaking up oven into more files is low hanging fruit. Is this something I could volunteer to work on? |
|
@behanw I'm interested in this too. I have actually re-written the whole thing in my fork but this has gotten more complicated in it's own way - it supports up to four zones and uses a React front end. If it's OK with Bruce, maybe we could create a branch in this repository and start refactoring. @jbruce12000 ? |
|
@chipgarner Since my previous post I've also rewritten most of the kiln-controller. I've used nicegui.io (which is based on FastAPI and Vue.js) and makes the UI absolutely trivial to work on, and pluggy to make it entirely plugin based, and as easy as possible to extend. I also have the beginnings to support an arbitrary number of thermocouples and SSDs. My setup also includes ambient temperature sensors, current sensors on the heating elements, status LEDs and an E-stop button. I've added a login system, https support, and completely overhauled the configuration system with the aim to allow it to be completely configured via the gui. I've also changed my firing profiles to be rate/temp/hold based like most other kiln controllers (and commercial PID controllers) I've used in the past. I'm also using tailscale to remotely access my kilns. I also have plans to add data logging to an sqlite database in order to be able to review what happened during a firing. At this point I believe I've diverged sufficiently from the original code base, that merging back is probably impossible. |
|
@behanw Nice. One thing I found really useful is a manual control mode. It's pretty easy to make ti so you can set the percent on manually from the UI for each zone. I mainly find it useful because I keep fooling around with model based control ideas instead of the PID . Also because I have had the MAX31855 and 31856 go haywire several times. I am very interested in your remote access. I am sending temperatures out via MQTT so I can view kilns online but I can't control them. |
|
Would love to give your software a try sounds like we have all been busy
I’ve upgraded the config to dynamic config json and html accessible from
frontend via button also made autotune automatically put figures in so no
more copy pasting stuff also added dynamic speed control into Java but
would love to give behanw s software a go will it work with a grounded
thermocouple for the max 31855 as that makes them accurate otherwise at
1000 degc they go out of whack by 16 degs c I still use jbruces older
software using the max drivers that came with it as they are accurate
grounded so far none of the new software handles this grounding it only
happens with max 31855 chip 31856 is ok and does not go out of span I have
a calibrator at the kiln factory and have tested this as a problem I’ve
been trying to also get going AI auto turning but would love to give yours
a try.if you need any programs in deg c I have the whole cone list in my
software they are only in deg c programs work good. Easy to convert them to
deg c in gpt chat.
cheers Brett from MeltTech.com.au
…On Fri, 8 Nov 2024 at 5:14 pm, Chip Garner ***@***.***> wrote:
@behanw <https://github.com/behanw> Nice. One thing I found really useful
is a manual control mode. It's pretty easy to make ti so you can set the
percent on manually from the UI for each zone.
I mainly find it useful because I keep fooling around with model based
control ideas instead of the PID . Also because I have had the MAX31855 and
31856 go haywire several times.
I am very interested in your remote access. I am sending temperatures out
via MQTT so I can view kilns online but I can't control them.
—
Reply to this email directly, view it on GitHub
<#131 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A2CVZ46RNP2SBB3EINBG7ITZ7RJDBAVCNFSM6AAAAABN5Q2DHSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINRTHA3DIMBXHA>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Refactor the code and add tests to make it faster, easier and less risky to add features to the code.
TLDR:
Break oven.py into 3 or 4 files.
Use abstract base classes to create virtual methods.
Minimize the use of "self." variables.
Write tests.
Goals
Readability: The easier it is to read and understand the code the easier it is to make changes. Most of the kiln-controller code is easily readable.
Modularity: Breaking the code into separate modules that interact in obvious ways make it easier to understand, modify, and test. Kiln-controller does this well and there are obvious functions, roughly:
Input schedule
Read temperature
Compute power
Command power
Display status
Automated testing: As code becomes more complex, automated testing becomes more valuable. Breaking something that already works when adding a new feature becomes more and more likely as code becomes more complex, and good tests prevent this. Kiln-controller has no automated tests, and adding then will require some changes.
Recommendations and thoughts
Break oven.py into more files. The oven file is very long and it is hard to find things. An example to put in a separate file is the code that reads temperature. Oven.py only needs to be able to read the best available temperature when it needs it. Any reading, error correction, averaging, etc. should be (and mostly is) in a separate class. Breaking oven.py into 3 or 4 files is easy and very low risk.
Use abstract base classes to create virtual methods. (Abstract classes in python can, and should, contain both virtual and concrete methods.) This makes it really obvious what the class does, and what methods need to be overridden. It also simplifies testing. This is also easy and low risk.
Pass variables to methods instead of using self. "self." should only be used when a variable needs to persist. These variables can be changed anywhere, which can lead to errors. The main reason to avoid using these to make them available to methods is that they make the method difficult to test. A method that takes variables, does something, and returns results is very easy to write unit tests for. This can be done piecemeal, especially when changing a method and adding unit tests. This is easy in many cases and low risk as most IDE's will flag any errors.
Pick a test framework. I like Pytest but any framework is fine.
Write unit tests. This requires some work. The parts of the code most in need of testing are currently not easy to test without modifying the code. It may be possible to create integrated tests that help with this. Unit tests need to run very fast and not be brittle!
Write integrated tests. The simulator can be used for testing some of the code. These can be slow but also need to be well thought out so they don't create a test maintenance issue.
Setup GitHub to run the tests on pull requests.
The text was updated successfully, but these errors were encountered: