@pastaclub how are we going to do the merge? about ureflowoven-esp32-micropython

[pastaclub]

Hi! I saw this issue only after already merging my transistor branch (which was only one commit and had no conflicts). If you want, we can revert it... but I think we have to start this way anyways since all my other changes rely on this commit.

I now created a new branch named repo-merge and merged all my other changes into that one. How about we merge everything in repo-merge and then merge it back to master?

Maybe we can merge gradually by cherry-picking single commits from beta into repo-merge.

[pastaclub]

Regarding the low-active heater, I think your solution with Signal is better and more compact, so we should use that approach. Maybe we can get rid of my Heater class altogether then. That said, I would like to use the terminology heater instead of oven. The controller is not restricted to ovens. I am using it with a hot plate, there is no oven involved, so to reduce confusion let's use the more universal name heater.

For config.json I vote to keep this structure:

    "heater_pins": {
        "heater": 21,
        "heater_active_low": false
    },

for the sake of keeping it consistent with tft_pins where the pin number and the low_active option are also properties of the same object (and not two seperate properties of the parent object).

[dukeduck1984]

Yes, let's try cherry-picking from beta to repo-merge, and eventually merge repo-merge back to master.

I agree that 'heater' is a more universal term and a better choice of word than 'oven'.

Also, yes to your proposed config.json structure.

This is the first time I'm involved in the collaboration on github - I'm not really familiar with cherry-picking, merge back and forth etc. All what I used to do was almost just one-way: change codes in PyCharm, then commit and push from there. I might need your help on this, and hopefully I don't break things :-)

[pastaclub]

For me it's the first time doing anything with Python :)

I rarely use cherry-pick since it's often easier to avoid having multiple people modify the same files at the same time, but it's not that different from regular merging:

1. Checkout the branch you want to merge into (i.e. repo-merge)
2. Find the number of the commit you want to merge from the other branch (i.e. beta). I use the github website to find it.
3. On the command line run git cherry-pick <commit-number>. This will apply the changes from that commit to your current state. If there are no conflicts, this will automatically result in a new commit. If there are conflicts, it will keep both the old and the new code in the affected files with the usual conflict markers
4. Have a look at what needs manual attention. I use the github desktop app to see local changes and it shows me which files are affected and where in the files the conflicts are. Use a text editor to fix manually and remove the conflict markers
5. Look at a DIFF again to see what has changed in total. Again I use github Desktop for this.
6. Commit as usual.

I already merged all changes to readme.md from beta to repo-merge today.

[pastaclub]

I also merged commit 8334338

[pastaclub]

I noticed that some things (especially in main.py) are getting tedious to merge into this direction, so I decided to merge some of my commits from repo-merge the other way into beta. This way I merged 5 commits, the largest of which was commit 3cd6c10, the others were relatively straight-forward.

This greatly reduced the differences between the two branches. Maybe we can eventually merge the rest in one go.

If we want to cherry-pick a bit more, we will have to start with the oldest commits since the newer ones depend on them. But I think I managed to merge already over 50%.

I'm going to an island tomorrow for 10 days or so. I may be able keep contributing a bit from there, but I won't have the hardware to test.

[pastaclub]

I managed to merge everything. We now have all code combined in the repo-merge branch!

...but I didn't have time to test yet. This merging involved a lot of manual work and it's possible that I made mistakes. Please have a critical look at it to see if anything is missing and if it works.

[dukeduck1984]

Wonderful! Thanks for the effort!

I will review the code and then test them on the hardware the coming week.

[dukeduck1984]

I have manually merged repo-merge to beta to reflect all the lastest updates with carefully side by side review. (I figured it's quicker to do it manually than to learn the git concepts I rarely used in this case)

So far the difference between beta and repo-merge is showed as following:

• config.json

  • "sensor_type": "MAX31855" - as I will test the code on my oven, so I kept the sensor type as what I use.
  • I removed "temp_offset", as it's replaced by "sensor_offset".

• heater.py

  • I have deleted this file as it's no longer used.

• main.py

  • File open is handled by context manager with open('file', 'r') as f: as it will automatically close the file afterwards.
  • A few blank lines according to PEP.

• oven_control.py

  • Line 182-191 has been removed, they are not needed (I have tested already).

• readme.md
  Some minor difference, except for:

  • You actually CAN power up the TFT screen with the GPIO directly, no transistor needed.
  • The temperature calibration no longer exsits, so I removed the description about that.

That's all. I will test the codes in beta on my oven tomorrow or the day after. The codes in repo-merge remain unchanged as you merged which can be a reference.

[pastaclub]

Your changes all make sense.

But I strongly suggest we use git merging the way it is meant to be used. PLEASE don't do what you called "manual merging"! This causes several problems:

• the differences which you intended to make will pop up as conflicts every time
• git will want to undo your changes anytime someone tries to merge the proper way
• we lose the whole commit history

In 'repo-merge' all the commits from both branches are retained, so you could pick any line of code and have git tell you when it was changed and why. All this information gets lost if you do your "manual merge" since there will just be one single commit containing the changes of all those 20 original commits.

Also, if you merge from branch A to B and during the merge you decide that there is a line in A which should not be in B, then by committing this as a proper merge, you are telling git that you have resolved a difference, and it will remember that and not bring this up again. If you just commit to B without designating it as a merge, git will always try to restore that line in B. Furthermore you have no way of seeing only new changes, because you can only look at a DIFF between A and B and you will keep seeing things you already resolved. If you had done a proper merge, you could keep merging back and forth easily and only see the new changes, but not the ones you already dismissed earlier.

If you don't want to deal with a git merge for now, the proper way to go would be to either keep working on the repo-merge branch, or create a new branch off repo-merge. Whatever you prefer of the two, you can commit the changes you mentioned there and it will avoid the problems.

[pastaclub]

To illustrate what I mean, let's say you wanted to know what the line "sensor_type": "MAX6675" in config.json is all about and how it got there. You can simply look at this blame report for config.json in repo-merge and it will neatly show you the commit messages for all lines and link you back to the original commit "Support for MAX6675 sensor" where you can also see what else was changed in other files when support for that sensor was introduced. In the beta branch, all this information is lost since the merge command was not used.

[dukeduck1984]

I completely understand what you meant and the concerns. I think I messed up the git thing on my side.

I tried to pull the remote repo-merge to merge with my local beta, but somehow it didn't show the conflict in PyCharm - my local beta just got overwritten. Then I tried reverting to recover my local beta before I merged again in the command line, this time the git said the beta was Already up-to-date, although I could see the difference between beta and repo-merge by git diff repo-merge. I was really confused and couldn't get it to work properly. That's why I decided to do it manually, but definitely you were right, it's not the right way to do it for the sake of collaboration.

So how can I fix this? As previously mentioned, right now the beta branch had been 'manually' merged, and the code in there is ready to be tested on hardware; The repo-merge branch remained unchanged as how you left it there on Jun.28.

I'm sorry for the trouble caused.

[pastaclub]

I fixed it by creating a new branch named beta2 and replicating your latest changes. Please check this branch.

I have split up your changes into a few seperate logical units:

• commit 39d8441 removes dead code
• commit 4fce105 changes the default sensor type to MAX31855
• commit ccfff45 restores context managers (sorry I broke this before. As I'm new to Python, I wasn't aware of what this actually does, until you explained it)
• commit ace8962 for formatting (basically just line-breaks)
• commit 8c53de0 updates readme.md

All the above is a replication of the changes from your "manual merge". The only thing I modified is the paragraph about the ACC pin in readme.md (please have a look).

You can keep working on the branch beta2 and commit as usual.

[dukeduck1984]

Super!!

[dukeduck1984]

I haven't had a chance to test it - too busy these days. I will post here as soon as I have done the testing.

[dukeduck1984]

I have uploaded the code in 'beta2' to my oven. The first thing I noticed was that the temp sensor reported error code 'X_PWR', 'X_GND', or 'ERR' in a random manner from time to time. In order to carry on the test, I had to comment out those lines in 'max31855.py'. Otherwise, it ran without an issue.

[pastaclub]

This sounds like an electrical problem in your hardware. I didn't add these error checks, I only changed the way the errors are handled and displayed.

I'd suggest to check your wiring (especially the SPI lines between the ESP and the MAX) and try if it goes away if you lower the SPI speed. It could also be related to power. If your MAX board doesn't already have them, you could try adding two capacitors between the VCC and GND pins of the sensor board (maybe 100nF and 10uF, something like that).

[dukeduck1984]

Yes, I thought the same - it's probably related to my hardware. I will try troubleshooting per your suggestion.

Besides, I think the temperature regulating performance was not very consistant - as I tuned all the params against profile 'Lead 183' which was the solder paste I was using, the temp control went very well as shown in the picture I posted. However for 'Lead 138', there was quite a bit of overshooting which I guess was caused by the too high value of 'preheat_until', while for profile 'Lead 217', the temp couldn't reach the peak of the ideal profile, it topped at 222 degree - I need to look into it, the temp control before the reflow stage was pretty good though.

I'm on business trip now and won't be back until Sunday. I will try to look into the temp control issues next week.

So far, I'm fine with merging the beta2 into the master, but if you get a chance to also test it before doing so, it'll be great.

Kaiyuan

[BG7YWL]

你好，可以编译一份ESP32-WROOM-32模块的固件给我吗？
我尝试刷入firmware_idf3_generic.bin固件后搜索不到WIFI信号，串口提示报错，我使用的是ESP32-WROOM-32模块+ESP32-WROOM烧录座进行测试。

[dukeduck1984]

请另开一个issue，把报错信息贴一下。

[pastaclub]

I will test when I get back to Bangkok (maybe on monday, let's see). I'm still on an island.
Should we close this issue?

[dukeduck1984]

Sure, take your time.

I'm closing the issue.

[pastaclub]

beta2 merged back into master.