Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Master to blinka branch #163

Closed
captaincaden opened this issue Nov 21, 2023 · 68 comments
Closed

Master to blinka branch #163

captaincaden opened this issue Nov 21, 2023 · 68 comments

Comments

@captaincaden
Copy link

captaincaden commented Nov 21, 2023

I've gathered that updates and new features are no longer being added to the master branch that a good amount of people already have/had in place, that being said, I feel a guide or diagram that outlines specifically the input/output changes from the docs on the master branch on a raspberry pi to update to the blinka branch would be nice.

Information on how to get the software from the master to the blinka branch would be nice as well would be nice. (Maybe something in the docs that outlines how to get updates in general)

Just a guide that outlines the swap specifically from working master to the blinka, not starting from zero for the blinka branch. I really hate to change everything I've already done, and I was really hoping for a solution that would come that would prevent those hardware changes from being needed, but it seems that isn't coming, so something to make the swap easier would be beneficial for a lot of people, I think.

@captaincaden
Copy link
Author

Also, @jbruce12000 when I installed the blinka branch on an empty pi, I still needed to download RPi.GPIO which wasn't in the requirements.txt file for the blinka branch. It was needed by one of the blinka modules, and throws an exception for not having RPi. I needed to separately install it to get the blinka branch to work. Maybe just on my end, but something to look at for others when they install the blinka branch from a pi that doesn't have the master on it already.

@amahpour
Copy link

Hardware changes? What hardware changes need to be made when switching to the blinka branch?

@captaincaden
Copy link
Author

captaincaden commented Nov 23, 2023

The input/outputs are different from the pi to the thermocouple board from the current master to the blinka branch. Everything else is the same.

In my case I needed to change 3 wires, which were soldered for me. But the people who are using just pins and connectors it'd be a lot easier for. And depending how others have things set up, accessing the pi and removing all of the connections to relay(s) may be more or less difficult if you have to make changes somewhere away from the kiln.

I needed to move:
SCK from GPIO 22 to GPIO 11.
SDO from GPIO 17 to GPIO 9.
CS from GPIO 27 to GPIO 5.
I already had SDI on GPIO 10. (Max31856)

Happy to say that after an hour or so of planning and tearing the whole setup down and desoldering everything I finally am up to date and running in the blinka branch.

@amahpour
Copy link

Bummer. I designed a hat around master:
image
image

I guess that makes sense not to merge at this point. If you do merge can we tag master before doing so? CC @jbruce12000.

@chipgarner
Copy link
Contributor

@jbruce12000 My pull request #132 from last February allows using software pins. It only includes the 31855 as I could not get it to work for the '56.

It first looks for the hardware pins and switches to software pins via Bitbang if nothing is found. I tried everything I could think of to get this to work with the '56 but it would not work with the new Adafruit libraries

Most users are probably using the '55.

The '56 does work, as in the Master branch, with the old Adafruit libraries. It would be pretty easy to make the code switch to the old library in the case of a '56 not using hardware pins. This will work only on the Pi, but only people using the Pi would have already soldered up to the software pins. This could only be applicable to a small number of people, such as zero.

@captaincaden
Copy link
Author

@chipgarner I'm up to date on the blinka branch now, using 31856 on a Pi Zero 2W. I switched all of the thermocouple pins to the hardware pins and it's currently working great.

I think having the option to use the software pins to make the transition easier for those already running the master would be a good feature still though.

@chipgarner
Copy link
Contributor

@amahpour It looks like you are a low level guy and may understand the MAX 31855 1nd 6 data sheets and firmware better than I do. Email me if you might like to help with this. Chip at garner followed by total followed by energy dot com. The three words after the at run together.

@jbruce12000
Copy link
Owner

ok, blinka autodetection now works for 31855 hw and sw spi... and 31856 hw spi.

I need one person to test 31856 sw spi. @chipgarner has a 31856 being shipped now... but I'll take feedback from anyone willing.

@captaincaden
Copy link
Author

@jbruce12000 I waited for a while to convert to blinka, but I really wanted the seek start feature. I had a situation a week or two ago while I was candleing the kiln then firing right after where the seek start would have made things a lot a lot faster, so I just decided to go ahead and do it. I understand you don't work on this 24/7, and as always, we're all appreciative of the time you and everyone else does spend on it.

I'd like to help with the software spi testing on 31856, but I'm assuming that requires the pins to be changed, which with my setup would be difficult. I'm going to update to your most recent commit on blinka now though.

@captaincaden
Copy link
Author

captaincaden commented Dec 1, 2023

Also @jbruce12000 did you see my comment about when I installed the blinka branch on an empty pi, and still needing RPi.GPIO during the install?

@jbruce12000
Copy link
Owner

issues:

  • docs for transition from old master to new master (what is blinka for now)
  • add RPi.GPIO to requirements.txt

@jbruce12000
Copy link
Owner

RPi.GPIO added to requirements.txt @captaincaden

@jbruce12000
Copy link
Owner

no worries @captaincaden re testing 31856. DO NOT desolder to test this. Someone else can do this when they convert. I think @amahpour is a good candidate since re-wiring would be REALLY tough for him. Not sure if he's running 31855 or 6 though.

@captaincaden
Copy link
Author

Updated to current commit on blinka. Appears to be working as expected on hardware spi for 31856.

@amahpour
Copy link

amahpour commented Dec 4, 2023

I’m on the 55. Should I test with that?

@jbruce12000
Copy link
Owner

@amahpour I have already tested software SPI with the 31855. If you need any of the features of blinka, you can switch when you like.

The only thing I have left to do before I switch blinka for the master branch is documentation about how to migrate from old to new.

@chipgarner
Copy link
Contributor

I am seeing the same issue as before with the MAX31856 and software SPI.

I am using a breadboard and connecting a Pi 3b+ to the '56. I am using a thermocouple simulator from @rondoc. The latest blinka branch of kiln-controller reads the temperatures correctly when connected to the hardware pins.

I then modified oven.py to skip checking for the hardware pins and run using bibangio, so it is using the hardware pins as software pins. This results in continuous errors:
Problem reading temp cold junction range fault
Problem reading temp thermocouple range fault

I then put oven.py back as it was, connected the Pi via gpio pins, and modified config.py to the right pins for software SPI.
I get the same to errors repeated as above. I also tried it intentionally using the wrong pins. It does not produce the errors above, and if all the pins are wrong it just uses 32f as the temperature.

I am pretty convinced I didn't make a pin error, and this is the same thing that I have seen previously when trying to use the MAX31856 with software bins via bitbangio in Circuit Python.

@chipgarner
Copy link
Contributor

Software pins appear to work with Arduino. It would should be possible to call the Adafruit Arduino library (it's in C or C++) from Circuit Python. We might only need to call one or two functions.

@Linkeor
Copy link

Linkeor commented Dec 8, 2023

Bonjour de France,
Thank you for this superb project.
I will present my project on occasion if you are interested but for now I would like to share with you my experiences on kiln-controller debian blinka on raspberry pico with u2if, and a max31856.
With busio/hardware pin, impossible to do anything (and I didn't look further), except get temperatures) 0 and no error the values recorded (0) are within the thresholds.
With bitbangio/software pin, on or outside the "hardware pin", I have the TC and CJ temperature which seems correct but impossible to write the threshold temperatures correctly, and as a result multiple errors appear
After multiple tests I put my finger on something in the bitbangio library and now everything works, I can write the thresholds and thus no longer have errors.

I put a message to this effect on the Adafruit_CircuitPython_BitbangIO github

adafruit/Adafruit_CircuitPython_BitbangIO#20 (comment)

I stay with you

@jbruce12000
Copy link
Owner

@Linkeor oh how exciting! We have a confirmed Raspberry PI Pico user.

I want to make sure I understand everything you said so I can understand what action I need to take, if any.

  1. are you using the lastest blinka branch code?
  2. you tested hardware spi with no problems or issues
  3. you tested software spi, but setting thresholds failed. You need to be able to set thresholds to define the min and max temp your thermocouple can read (for error checking). You found a bug in the Adafruit_CircuitPython_BitbangIO code that had already been reported, but not merged, so you fixed it locally.

So now, both hardware and software SPI have been proven to work on the Pico using U2IF.

@chipgarner
Copy link
Contributor

@Linkeor Thanks! I will test this with my MAX31856 and MAX31855.

@chipgarner
Copy link
Contributor

Woohoo! I am reading temperatures using software pins on the 56. I need to make sure this does not break the '55, the method is called from many places in the library. Thank you for finding this @Linkeor

@chipgarner
Copy link
Contributor

Good news!

I tested the simple changes from @Linkeor with the 55 and 56. Both work fine in using both the software and the hardware pins. So, all 4 cases. This was with mi Pi 3 and a breadboard, using both a thermcouple and the TC tester.

@jbruce12000 The hardware pins in config.py will only work on the Raspberry Pi. If you use MOSI etc they will work on all the blinka supported boards. People only need to set pins to GPIO numbers if they are using the software pins.
spi_sclk = board.SCK #spi clock
spi_mosi = board.MOSI #spi Microcomputer Out Serial In (not connected on MAX315855, needed on MAX315856)
spi_miso = board.MISO #spi Microcomputer In Serial Out

@chipgarner
Copy link
Contributor

@jbruce12000 You need to decide how you want to deal with the library change. I think you can just include the changed version in git with the project and it will override the one in the lib. If you fork the lib you will be notified of changes and can more easily keep things up to date.

@chipgarner
Copy link
Contributor

@Linkeor Someone should submit a pull request to Adafruit for the library. You should get the credit!

I can try to submit a request if you don't want to.

@Linkeor
Copy link

Linkeor commented Dec 8, 2023

@jbruce12000
Unfortunately, I am not on the very latest version of the blinka branch, I am two weeks late because initially I had translated the files into French because the artist is not me but my mother...
For here Christmas I want to present him with the nicest model possible. There are two ovens to equip.
We're not going to teach him English at his age...
What I want is to stop getting up several times a night to check the temperature on her old controller.
So, I haven't tested the autodetection but I won't fail to do so. I don't think this will be a problem because I can use the pins with their number or with their name (sck1 miso1 mosi1 for my part). Bitbangio always remains a software pin even if we use the "hardware" spi port. We are not at all obliged to use the pins “dedicated” to the spi for everything to work. I tried.
It seems to me that there is a confusion between hardware pin and hardware mode.
As I can't get anything done with busio, which is the real hardware mode(with hardware pin), for my part, I can't confirm that the 56 works in hardware mode on the pico/u2if.

yes I have fixed the library locally, I wrote a short message on the ussue which seemed most appropriate to me, I would like the opinion of the parents of the library who will be able to judge the possible side effects. My modification may have to be part of a particular case and therefore of a test (if blablabla) For me it also works for a tft in phase 0, I remain confident.

@chipgarner
I don't think this will affect the max31855 because I touched def write() from the bitbangio library and mosi is not necessary on the 55, so there is nothing to write...

ps: sorry for Google translate (je parle anglais comme une vache espagnole)

@Linkeor
Copy link

Linkeor commented Dec 8, 2023

I just posted, I discovered your intermediate messages. I'd like to make the pull request and get the credit, but I have to wait until Monday.

@Linkeor
Copy link

Linkeor commented Dec 8, 2023

after re-reading, I think that clarification is necessary for 56. I used the library's git test script to work. The 56 returns an error if the values ​​read are not within the defined thresholds.
This time it's hardware error management...
It is therefore necessary to set them according to our hardware when launching the application. I think you will have to think about determining them in the config.py and initializing them somewhere (one tuple for the tc, one for the cj). See later in which scenarios it is interesting to exploit this functionality.
Another clarification, I haven't tested anything full-scale yet and the thermocouple I use on the breadboard is the cheapest I found on Amazon. Those in the ovens remain where they are now.

@chipgarner
Copy link
Contributor

@Linkeor I have played with the thresholds in the past but have not needed to change them on the '56 at temperatures up to about 1200C. I think the defaults are adequate. Let me know if you think I might be missing something.

You are right, the sensors will run using hardware pins and the bitbang software SPI. I went back and tested, the '55 and '56 work in all the expected modes: Hardware pins - software SPI via bitbang, hardware pins - hardware SPI, Non-hardware pins - software SPI via bitbang.

@jbruce12000
Copy link
Owner

jbruce12000 commented Dec 11, 2023

@chipgarner code and documentation changes completed on blinka for NOT listing HW SPI pins in config.py.

If you can, please test 31855,31856 HW and SW spi.

The code assumes SW spi if you have pins in config.py. Testing by commenting out pins in config.py and restarting should be pretty quick.

./test_thermocouple.py was also changed and it'd be handy to test that.

@Linkeor
Copy link

Linkeor commented Dec 11, 2023

git fetch right now on the blinka branch

  • Configuring the config.py with SCLK1, MOSI1 and MISO1 + max31856 ==> print software SPI as expected and everything is fine.

  • Out of awareness but without really believing in it because the manual configuration of the spi hardware does not work on pico and spi.board() only calls busio, I commented spi_sclk=, spi_mosi= and spi_miso= ==> print hardware SPI but KO temp =0 as expected with test_thermocouple.py

  • rewiring on SCLK0 MOSI0 MISO0 (default hardware spi port on pico), same result.

The autoconfiguration script seems to work really well but for my part, only the spi software works.

@jbruce12000
Copy link
Owner

@Linkeor Thank you for testing.

@chipgarner
Copy link
Contributor

I am hitting errors with the software SPI in kiln-controller but test-thermocouple is working fine. This is odd because the code looks the same! I am exploring further.

@chipgarner
Copy link
Contributor

The reason test-thermocouple works is because it is not checking for errors. It returns the correct temperature. The errors are range errors.

@jbruce12000
Copy link
Owner

Yes, test-thermocouple is specifically ignoring errors. OK, sounds like no code changes are needed. I need to decide what to do with the bitbangio lib.

@Linkeor
Copy link

Linkeor commented Dec 12, 2023

@chipgarner You should test with the max31856 test script and see the written and read values of the different thresholds. Remember to modify them in the .py because the thresholds in the scripts are examples and do not correspond to the thermocouples used and could quickly be exceeded...

@jbruce12000 I could be wrong but I really don't think that the problem is the modified code because the problem corrected is of a lower level (problem of writing the last bit of each byte due to a "half phase" shift --> no writing on the last rising/falling edge --> wrong values written)
I never had an error with test-thermocouple.py even without modifying bitbangio. With kiln-controller.py and https://github.com/adafruit/Adafruit_CircuitPython_MAX31856/blob/main/examples/max31856_thresholds_and_faults.py

At what temperature does the error occur? (ambient? heated?)

Have you had the opportunity to read #165 ?

@Linkeor
Copy link

Linkeor commented Dec 12, 2023

@Linkeor
Copy link

Linkeor commented Dec 12, 2023

@chipgarner thank you very much for the support on the pull request adafruit/Adafruit_CircuitPython_BitbangIO#30

@rondoc
Copy link

rondoc commented Dec 12, 2023 via email

@chipgarner
Copy link
Contributor

chipgarner commented Dec 13, 2023

So, after getting all the pins right, both the 55 and 56 are working with kiln_controller.py and test_thermocouple.py. I tested all the combinations of hardware SPI and hardware and software pins, including software SPI with hardware pins. They all work.

The range errors ware apparently from a pin mismatch, nothing to do with the threshold settings.

@jbruce12000 My test environment was using the edited library as requirements.txt will load the one with the error. Let me know if you intended something different.

@chipgarner
Copy link
Contributor

PS I tested at ambient and with a little heating with a heat gun.

@Linkeor
Copy link

Linkeor commented Dec 13, 2023

Hello everyone,

@chipgarner that’s reassuring :-)

@jbruce12000
In my previous message, I think I misunderstood and responded off topic, another language problem for me, and I apologize for that.
Also, if I may allow myself my vision of things:

  • The code seems to be all good and should no longer be reworked in view of the migration.

  • Concerning the bitbangio library, users of 55 are not impacted by the original library because nothing is written in the register (everything was stable for max55 before modification of bitbangio? Do we agree? ).
    Also, you should not include the modified library in the project. I rather think that it is necessary to add in the instructions for 56 users the slight modifications to consider for themselves while waiting for a merge of the pull request or an adaptation on the part of the developers of the library. It is also possible to create a script to make these changes.

  • Users should also be warned that the support for the 56 is not necessarily optimal and may still be under development.
    In this regard, after re-reading the '56 datasheet and the code of the 56 library, it seems that there are functionalities and ways of doing things that are not implemented by the max31856 library. The pull-request Added SPI baudrate argument, high res temperature reading, and autoconvert function adafruit/Adafruit_CircuitPython_MAX31856#28 is proof of this. We also see that the library only uses oneshot measurement (or I missed something). The use of 56 is therefore still basic and its library does not allow the exploitation of its full potential (but do we need it?)
    For users who don't want/can't change anything and don't want to migrate (that will be sad), you should keep the old "main" branch under another name to allow easy access if needed.

Nice work to you, thank you again :-) Fire!

@jbruce12000
Copy link
Owner

I likely have a few more commits for documentation changes, but no more code changes unless someone finds a bug.

Yes, I'll keep the old master branch contents and history around under a different name so that folks can continue to use that if the choose not to migrate/upgrade.

@chipgarner
Copy link
Contributor

@Linkeor is right, just add instructions for now for editing the library. It is a very easy edit and it is easy to find the file in the virtual environment folder. Hopefully they will merge the pull request soon.

@captaincaden
Copy link
Author

captaincaden commented Dec 16, 2023

I haven't updated since the first blinka spi commits (2 weeks ago). But I'm having some odd thermocouple issues today (31856, hardware pins). Temp seems to be jumping around intermittently, and connection to the pi is extremely slow for some reason today. I fired yesterday with no issues, and have fired two other times since updating with no issues, so I'm not exactly sure what's going on currently, or if any of the most recent commits will solve whatever's going on. I'm continuing the firing currently, and I can't connect to the pi to check logs right now because of how long it's taking to connect it keeps timing out. Takes 2 minutes or more sometimes to load the websocket page today for some reason (html and elements show up pretty quick, but actual live current data is taking time), and ssh keeps timing out when I try to connect to it while it's firing currently.

I know that the issue is happening in the back end too, as I have it set up to send me an email when the temperature runs away from the target temperature within a certain amount (gets the data to do this in oven.py) , and I got the email that it jumped from 1597 ish degrees to 1615 within a minute and then back to around what I'd expect temperature by the time I refreshed the page. Once this firing completes I'll take a look at the logs if I can get them to load and look into it. I'll update to most recent commits soon too.

Update, I've pulled the logs twice now from WinSCP, for some reason for the past 20-30 minutes I'm not seeing any of the normal logs from kiln-controller besides the get requests when I load the page in a browser. The kiln has still been running, and still functioning for the past 20 minutes, but it hasn't logged the normal string of temp and err and pid information it normally does when running. No exceptions or errors logged in daemon around either. I don't see any other errors or abnormalities in the logs other than the temperature being sporadic in places. I'm going to attempt to stop and reboot and restart the firing.

Reboot seems to have fixed the delay in loading the websocket. Can access logs and ssh at normal speeds again. Kiln is continuing to fire and behaving normally for now. Will continue to watch for thermocouple issues.

@captaincaden
Copy link
Author

captaincaden commented Dec 16, 2023

In the process of updating to the current blinka branch. Personally, I find the auto detection for hardware/software confusing, at least at this point. It just makes more sense to have everything spelled out in config to me. Having issues updating now (edited, seems to be working now, with information below):

@jbruce12000 what are we supposed to put in place if we're using hardware pins in the spot where the pins are currently listed in config? I originally left it as it was with the hardware pin values I had before, and I just got a cold junction error (was detecting software SPI). I then commented out all of the pins besides CS (assuming it needed to see nothing on the other pins.) This worked, and detects hardware, but now my thermocouple isn't reading. I just get a constant 32 degrees f in both test-thermocouple.py and kiln-controller.py.

Put everything back to how it was in config, oven, and test-thermocouple, and still only getting 32 degrees f now. Confirmed everything is back to how it was, still see 32.00 consistently.

UPDATE: Rebooted, and am getting the right temp now. Still showed 32.00f every time immediately before reboot. I'll try to put everything back to most recent commits again and see if it works or not now. Any idea why this would happen? I'm not understanding why rebooting the pi would have fixed this.

updated again to most recent blinka commit (commenting out the first 3 tc pins in config) and seems to be reading normally now. Still would like some insight on the problems I had earlier today during my firing, and the problems relevant to this comment if anyone has any.

@chipgarner
Copy link
Contributor

Could all @captaincaden 's issues be that two instances are somehow running at the same time? This might cause issues with both temperature readings and websockets.

@captaincaden
Copy link
Author

@chipgarner would there be a way I could check for that? I had also noticed after I updated last time about two weeks ago or so, I was occasionally getting the exact same degree and decimal on the thermocouple every so often. That appears to not be happening anymore currently.

Example with random numbers:
Normally my thermocouple readings each time it's called to was something like
34.057483934
34.029588293
33.938838383
and continuously changing each time, even if it was just a little bit, which I believe is normal.

But after I updated and restarted it last time I was frequently getting things like:
34.057483934
34.029588293
33.938838383
33.938838383
33.938838383
33.938838383
32.389020202

No idea if it's relative or not, but like I said, after everything I did today, I'm noticing that's not happening anymore, which again, I think is how It's supposed to be, so not sure if what had been happening is relative to this or not.

@chipgarner
Copy link
Contributor

The websocket code was updated about two weeks ago, do you know if your issues were before or after this? This doesn't explain the temperature spikes.

The two instances issue is not very likely unless you did something like I seem to be good at, like forgetting you set this Pi up to auto-start last year and then starting kiln-contoller in a terminal...

Some repeated temperatures may be normal. The MAX31856 take about 0.7 seconds to compute a new temperature and it falls behind if temperatures are queried faster.

@captaincaden
Copy link
Author

I know I fired on December 7th, and 8th, and like I said I fired yesterday as well with no issues. Which was after I updated to blinka commit e0d2aac

Curious, does sudo systemctl start kiln-controller start a new instance if one had already started on boot? If so, I've definitely done that on more occasions than not.

That being said, I'm pretty certain I didn't start another instance while I was updating and having the 32 degrees f issue. If I did, I would have done it when I rebooted too, because I would have done it from inside of cd kiln-controller to see test-thermocouple.py

@chipgarner
Copy link
Contributor

Checking, I don't think there is protection on starting a second instance.

@chipgarner
Copy link
Contributor

I can't start a second instance on the pi, it hits:
OSError: [Errno 98] Address already in use: ('0.0.0.0', 8081)
if an instance is already running and using that port.

It looks like it would be pretty difficult to start a second instance of kiln-contoller.py.

@captaincaden
Copy link
Author

@chipgarner good to know. I have it running another firing at the moment, and it's behaving as expected currently. So, we'll see how it goes.

@captaincaden
Copy link
Author

Firing finished with no issues or abnormalities now.

@jbruce12000
Copy link
Owner

@captaincaden I'll provide explicit instructions on what to do in the case of HW or SW spi when I move blinka over to master.

As it stands, if you're using HW SPI, all pin assignments in config.py should be commented out.

@jbruce12000
Copy link
Owner

My brother had a stroke so I wont likely get the blinka branch moved as soon as I wanted. I have to go to the hospital every day and manage his care, so busy.

@captaincaden
Copy link
Author

@jbruce12000 So sorry to hear about your brother. Best wishes

@jbruce12000
Copy link
Owner

The main branch was created today from blinka and it is set as the default branch. The master branch still exists, but it is no longer maintained and should be avoided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants