11:34 <mtrbot-ml> [mattermost] <hartytp> @astro comments on Thermostat

11:36 <mtrbot-ml> [mattermost] <hartytp> 1. Since there is quite a bit of overlap between Thermostat and Stabilizer on a hardware/use level, I'd really like to see as much overlap as conveniently possible between the codebase, tooling and user interfaces

11:37 <mtrbot-ml> [mattermost] <hartytp> 2. I don't mind nix (and I get why you want it for your CI system) but it's really great if the firmware can be built without nix. I assume this should be okay now that we're no longer using an old version of smoltcp?

11:38 <mtrbot-ml> [mattermost] <hartytp> 1a. For obvious reasons, the v1.0 firmware was heavily based on IonPak, but it would be good to do a little de-krufting and remove any residual ion-pak bits that aren't relevant to thermostat (unless that's already been done since last time I looked at the code)

11:39 <mtrbot-ml> [mattermost] <hartytp> I still generally prefer git hub to the m-labs gitea

11:41 <mtrbot-ml> [mattermost] <hartytp> from a user perspective, it's nicer to keep everything in one place. Also last time I checked, there were annoyances with the m-labs system like password reset not being enabled. these things can be fixed relatively easily, but I don't see the m-labs system being preferable from a user perspective. It's a weak preference and I'm not going to kick up a fuss over it, but I'd still prefer to be on GH

11:46 <mtrbot-ml> [mattermost] <hartytp> also, let's move it to a proper "thermostat" repo rather than "ionpak-thermostat" (there is no connection between those projects...)

11:49 <mtrbot-ml> [mattermost] <hartytp> 3. The way you're doing the maths for the temperature conversion right now is prone to rounding errors. That will definitely limit the overall system performance. There are two approaches to resolving this: be smarter with the maths (linearize around the operating point and take care with scale factors etc) or use f64

11:49 <mtrbot-ml> [mattermost] <hartytp> we need to do one or the other. f64 is probably easiest (that's what was in the contract)

11:50 <mtrbot-ml> [mattermost] <hartytp> how do I search within the project sources in gitea?

11:52 <mtrbot-ml> [mattermost] <hartytp> https://git.m-labs.hk/M-Labs/ionpak-thermostat/src/branch/master/firmware/src/main.rs#L262 shouldn't this be 0xFFFFFF (i.e. 24-bits, not 23 bits)? Or is this to do with the way the ADC is configured?

11:57 <mtrbot-ml> [mattermost] <hartytp> 4. The temperature conversion does not seem right

11:57 <mtrbot-ml> [mattermost] <hartytp> https://git.m-labs.hk/M-Labs/ionpak-thermostat/src/branch/master/firmware/src/steinhart_hart.rs#L23

11:57 <mtrbot-ml> [mattermost] <hartytp> I don't think that's the right way of calculating the thermistor resistance

12:02 <mtrbot-ml> [mattermost] <hartytp> https://pasteboard.co/IXZ7Kef.jpg

12:03 <mtrbot-ml> [mattermost] <hartytp> (and NB that beta is just the ADC word / 0xFFFFFF )

12:04 <mtrbot-ml> [mattermost] <hartytp> you shouldn't need an r.abs() since resistance is always positive

12:04 <mtrbot-ml> [mattermost] <sb10q> @hartytp the repos is here https://git.m-labs.hk/m-labs/thermostat

12:05 <mtrbot-ml> [mattermost] <hartytp> ok I didn't notice that one as the ionpak-thermostat once has been updated more recently

12:05 <mtrbot-ml> [mattermost] <hartytp> fine, well that addresses one thing :)

12:07 <mtrbot-ml> [mattermost] <sb10q> and password reset now works on gitea afaik

12:08 <mtrbot-ml> [mattermost] <hartytp> ok, that's new since last time I checked. Anyway, from a purely user perspective I can see no way that it's not less convenient that GH

12:09 <mtrbot-ml> [mattermost] <hartytp> For the Steinhart-Hart equation, the form I'm nost used to working with (and which maps best to the parameters that manufacturers tend to specify) is For the Steinhart-Hart equation, the form I'm nost used to working with (and which maps best to the parameters that manufacturers tend to specify) is :

12:10 <mtrbot-ml> [mattermost] <hartytp> I don't really mind how we do the maths in firmware since we can always add conversions to the python-side code

12:10 <mtrbot-ml> [mattermost] <hartytp> but that's just a FYI

12:10 <mtrbot-ml> [mattermost] <hartytp> Anyway, I'm pretty sure the temperature conversion doesn't work right now. The best bet would be to place a 10k resistor in place of a thermistor and check you can get T0 out of it

12:12 <mtrbot-ml> [mattermost] <sb10q> yeah, wait until GH deletes your account and the organizations you are associated with because you expressed an opinion that they dislike

12:12 <mtrbot-ml> [mattermost] <hartytp> 5. The TEC driver is now controlled by an 18-bit dac (AD5680BRJZ) rather than PWM

12:12 <mtrbot-ml> [mattermost] <sb10q> it's microsoft and they do stuff like that

12:13 <mtrbot-ml> [mattermost] <hartytp> although the current/voltage limits are still set by PWM

12:13 <mtrbot-ml> [mattermost] <hartytp> @sb10q I've never heard of GH doing that

12:13 <mtrbot-ml> [mattermost] <sb10q> I have and people I know have experienced it first hand

12:13 <mtrbot-ml> [mattermost] <hartytp> and would be happy to cross that bridge when we come to it. Right now I find GH easier to deal with and am more concerned with UX than making everything purely open source

12:14 <mtrbot-ml> [mattermost] <hartytp> well, I don't have time for this argument, I'm purely expressing my opinion as a user

12:14 <mtrbot-ml> [mattermost] <hartytp> 6. A low priority, but once everything else is working the contract calls for calibration of the zero-current point of the TEC driver.

12:15 <mtrbot-ml> [mattermost] <hartytp> This means using the microprocessor ADC to measure the TEC driver reference voltage and then figuring out which DAC word gives the same ADC reading

12:15 <mtrbot-ml> [mattermost] <hartytp> since we use the same ADC, gain, etc most systematics in the measurement should common-mode out and this should be pretty accurate

12:19 <mtrbot-ml> [mattermost] <hartytp> 7. You can read back the TEC current and voltage using the microprocess's ADC

12:20 <mtrbot-ml> [mattermost] <hartytp> I'd strongly suggest that for development/testing you hook a power resistor up in place of a TEC and use those to test functionality

12:20 <mtrbot-ml> [mattermost] <hartytp> e.g. check that the current limit functionality does actually limit the current etc etc

12:20 <mtrbot-ml> [mattermost] <hartytp> (it will take much less of everyone's time if you can test your code in place rather than relying on us to test for you and report back)

12:22 <mtrbot-ml> [mattermost] <hartytp> 8. We haven't funded software power/thermal management so it's left to the user to set current limits which won't result in over temperature/current issues

12:22 <mtrbot-ml> [mattermost] <hartytp> before this becomes a full production system, those issues will need to be addressed so worth bearing that in mind at this stage (even though we won't actually implement the features)

12:23 <mtrbot-ml> [mattermost] <hartytp> 9. Then the rest of the comments I have were about making the interface a bit tidier

12:24 <mtrbot-ml> [mattermost] <hartytp> The more I think about it, the more I'd suggest having the simplest possible remote interface, which only works in machine units

12:24 <mtrbot-ml> [mattermost] <hartytp> and then add SI conversion functions to the python driver

12:25 <mtrbot-ml> [mattermost] <hartytp> anyway, I can't remember where we are with this, so I suggest the easiest thing is if you sketch out what the python driver should look like first

12:25 <mtrbot-ml> [mattermost] <hartytp> we can then give any feedback on that

12:25 <mtrbot-ml> [mattermost] <hartytp> @pathfinder49 any other comments?

12:26 <mtrbot-ml> [mattermost] <hartytp> @astro other than that, if you have any questions about how the hw works (e.g. what voltage to apply on which pin to do what, how to configure pin straps, etc etc) just let me know

12:26 <mtrbot-ml> [mattermost] <hartytp> oh, if it's easy let's also expose things like the TEC driver frequency control

12:27 <mtrbot-ml> [mattermost] <hartytp> or, if you don't want to do that, please at least make sure we document what setting is used

12:27 <mtrbot-ml> [mattermost] <hartytp> same goes for the ADC filter settings etc

12:28 <mtrbot-ml> [mattermost] <hartytp> I'm happy with a methodology where we start with a minimal user interface and only expose settings as they are required by users (it's trivial to add to the UI) but it is nice to have a clear documentation of the important bits of hw configuration (sample rate, bandwidth etc)

12:28 <mtrbot-ml> [mattermost] <hartytp> Thanks! Looking forward to playing with it

12:30 <mtrbot-ml> [mattermost] <hartytp> the other thing is that we should maintain a to do list of things that aren't yet implemented (AT detection, thermal management, limiting the combined power on the two channels, etc)

12:30 <mtrbot-ml> [mattermost] <hartytp> maybe just gitea issues?

12:31 <mtrbot-ml> [mattermost] <hartytp> 10. also, adding LED control to indicate the regulation status etc

12:32 <mtrbot-ml> [mattermost] <hartytp> 11. the PID controller does "gain after integrate" so a small change in the gain will result in a spike in the output, which is nasty

12:33 <mtrbot-ml> [mattermost] <hartytp> should be integral += error*ki

12:33 <mtrbot-ml> [mattermost] <hartytp> cf the loop filter in stabilizer

12:35 <mtrbot-ml> [mattermost] <hartytp> 12. Maybe it's not an issue for these SD ADCs, but it's generally best practice not to continually poll the ADC

12:35 <mtrbot-ml> [mattermost] <hartytp> doesn't it have a pin that indicates when data is ready?

12:42 <mtrbot-ml> [mattermost] <sb10q> what is the issue with polling? the wasted CPU cycles? the EMI?

12:42 <mtrbot-ml> [mattermost] <sb10q> the power consumption?

12:43 <mtrbot-ml> [mattermost] <hartytp> https://github.com/sinara-hw/Booster/issues/325

12:43 <mtrbot-ml> [mattermost] <hartytp> things like that

12:43 <mtrbot-ml> [mattermost] <hartytp> likely not an issue with SD ADCs I guess, but it's generally better not to

12:44 <mtrbot-ml> [mattermost] <hartytp> since the ADC provides a DOUT/RDYn pin that already goes to a GPIO on the micro, it's really easy to avoid doing that

12:44 <mtrbot-ml> [mattermost] <hartytp> you can hook it up to an interrupt if you want to be fancy, but otherwise just poll that GPIO

12:45 <mtrbot-ml> [mattermost] <sb10q> hmm why would that apply to thermostat?

12:45 <mtrbot-ml> [mattermost] <sb10q> and it seems greg did not fix the underlying problem but merely applied a workaroud

12:45 <mtrbot-ml> [mattermost] <hartytp> well, 3.3V / 24 bits is a pretty small voltage

12:45 <mtrbot-ml> [mattermost] <sb10q> the ADC isn't supposed to behave like that during ADC transactions, is it?

12:45 <mtrbot-ml> [mattermost] <hartytp> it's pretty easy to get artifacts due to noise on the digital side

12:45 <mtrbot-ml> [mattermost] <sb10q> *I2C transactions

12:45 <mtrbot-ml> [mattermost] <hartytp> no, Greg fixed the underlying problem which is that it's not recommended practice to read out from those ADCs when they are mid conversion

12:46 <mtrbot-ml> [mattermost] <sb10q> recommended by whom?

12:46 <mtrbot-ml> [mattermost] <hartytp> datahseets

12:46 <mtrbot-ml> [mattermost] <hartytp> that's pretty standard

12:46 <mtrbot-ml> [mattermost] <hartytp> depends on ADC architecture, but there are windows in the cycle where they are very noise sensitive

12:47 <mtrbot-ml> [mattermost] <sb10q> and they come with a I2C bus which is potentially shared with many other devices? sounds stupid

12:50 <mtrbot-ml> [mattermost] <hartytp> anyway, this again falls into the category of things I don't have time to argue about. My point is just (1) it's generally a good idea to minimize activity on the digital lines during ADC conversions (2) polling a GPIO is as easy as polling an ADC register, is more in line with standard usage/recommendations and is quicker than arguing about ADC design

12:53 <mtrbot-ml> [mattermost] <sb10q> arguing about those things is where good design comes from :) something point-to-point like SPI would have been more appropriate than I2C, which is designed for shared communications (even supports several masters in theory), and becomes a footgun in this case

12:54 <mtrbot-ml> [mattermost] <sb10q> it probably doesn't matter at all in our case, but it's not a good decision from the chip vendor

12:54 <mtrbot-ml> [mattermost] <hartytp> sure. For booster, there are a few issues like that. However, they're not thermostat-relevant

12:55 <mtrbot-ml> [mattermost] <sb10q> and one can do point-to-point I2C of course. but a I2C bus is normally shared.

12:57 <mtrbot-ml> [mattermost] <hartytp> actually, to be fair, in booster the problem was even worse

12:58 <mtrbot-ml> [mattermost] <hartytp> because they didn't do any synchronisation between the read out and the conversion

12:59 <mtrbot-ml> [mattermost] <hartytp> so the two data words that were read out could come from different samples, which can cause large glitches

12:59 <mtrbot-ml> [mattermost] <hartytp> IIRC the thermostat firmware currently ensures we only read out each sample once

13:00 <mtrbot-ml> [mattermost] <hartytp> but, it still feels cleaner to time everything from the DOUT line as that's what it's there for

13:03 <mtrbot-ml> [mattermost] <sb10q> you can also say that polling follows the principle of economy of mechanism since everything goes over the same interface

13:05 <mtrbot-ml> [mattermost] <sb10q> it also serializes everything whereas the DOUT line operates in parallel (though the race condition problems are moot in this case)