review (firmware-hardware) vs (Thermostat design goals) #24

Closed
opened 2020-12-09 00:49:54 +08:00 by jbqubit · 5 comments

BTW I would definitely not consider this firmware production ready yet. Last time I looked over it the firmware did not do what I expect in various places (e.g. the zero-current calibration looked wrong). This isn’t yet at the point of being small bugs that need fixing, but rather misunderstandings over how the circuit/firmware should operate which will lead to surprising results.

IMHO it would not be a good use of anyone’s time to try to debug/improve this by filing issues about how the observed behaviour deviates from expectations; that will just lead to time wasted on XY problems. The firmware needs a full line-by-line design review from someone who understands how the circuit + firmware are supposed to operate and until that’s happened I would discourage people from trying to use it. (To be clear, the firmware is nicely written and it’s mainly there, there are just some misunderstandings which will cause pain. The changes to fix these will be small measured in terms of lines of code, but the review needed to find them will take a lot of time from someone with the right skill base...)

#11 (comment)

@hartytp I guess you're the only one who can do this review. This Issues is a reminder that it needs to be done at some point. It also lets prospective users of Thermostat know that such a review has not yet been conducted.

BTW: With the bug fixes that @astro has done in the last couple days I've found Thermostat is working well enough for some limited purposes in my lab. Many thanks to everyone for the quick edits and communication on the several (many!) Issues I've posted. I'm glad to be using Thermostat in my lab and looking forward to relying on it for robust temperature control down the line.

> BTW I would definitely not consider this firmware production ready yet. Last time I looked over it the firmware did not do what I expect in various places (e.g. the zero-current calibration looked wrong). This isn’t yet at the point of being small bugs that need fixing, but rather misunderstandings over how the circuit/firmware should operate which will lead to surprising results. > IMHO it would not be a good use of anyone’s time to try to debug/improve this by filing issues about how the observed behaviour deviates from expectations; that will just lead to time wasted on XY problems. The firmware needs a full line-by-line design review from someone who understands how the circuit + firmware are supposed to operate and until that’s happened I would discourage people from trying to use it. (To be clear, the firmware is nicely written and it’s mainly there, there are just some misunderstandings which will cause pain. The changes to fix these will be small measured in terms of lines of code, but the review needed to find them will take a lot of time from someone with the right skill base...) https://git.m-labs.hk/M-Labs/thermostat/issues/11#issuecomment-1212 @hartytp I guess you're the only one who can do this review. This Issues is a reminder that it needs to be done at some point. It also lets prospective users of Thermostat know that such a review has not yet been conducted. BTW: With the bug fixes that @astro has done in the last couple days I've found Thermostat is working well enough for some limited purposes in my lab. Many thanks to everyone for the quick edits and communication on the several (many!) Issues I've posted. I'm glad to be using Thermostat in my lab and looking forward to relying on it for robust temperature control down the line.

It really shouldn't have to be me who does the review. The thermostat EEM project is live and I'm hoping we can unify the firmware from the two projects to create a single repo that works nicely...

It really shouldn't have to be me who does the review. The thermostat EEM project is live and I'm hoping we can unify the firmware from the two projects to create a single repo that works nicely...

Curious who besides Oxford, oxionics and I are using Thermostat?

Curious who besides Oxford, oxionics and I are using Thermostat?

No idea. I assume no one (and none of us are using it yet for the reasons discussed), but I might be wrong.

It may well be useable to some degree right now, but it's not really tested and seems to do a few quirky things so I wouldn't want to rely on it...

No idea. I assume no one (and none of us are using it yet for the reasons discussed), but I might be wrong. It may well be useable to some degree right now, but it's not really tested and seems to do a few quirky things so I wouldn't want to rely on it...
topquark12 was assigned by sb10q 2020-12-28 17:51:57 +08:00

@hartytp Are you happy with the state of things after the latest changes?

We're getting ~300µK pk-pk (self-reported) stability here over 12 hours, and the other features also seem to work mostly fine (except for the reported issues).

@hartytp Are you happy with the state of things after the latest changes? We're getting ~300µK pk-pk (self-reported) stability here over 12 hours, and the other features also seem to work mostly fine (except for the reported issues).

I'm definitely happy with the recent progress. I'm afraid I haven't looked at the code/tested it on hw recently so can't comment on the actual state of things. FWIW, overall it wasn't in bad shape before, just a few things that didn't seem right. AFAICT (from following the issue) all of the things on my list are now resolved, so it's quite possible everything is working nicely now. Glad to hear you're getting good stability with it...

I'm definitely happy with the recent progress. I'm afraid I haven't looked at the code/tested it on hw recently so can't comment on the actual state of things. FWIW, overall it wasn't in bad shape before, just a few things that didn't seem right. AFAICT (from following the issue) all of the things on my list are now resolved, so it's quite possible everything is working nicely now. Glad to hear you're getting good stability with it...
sb10q closed this issue 2021-01-25 20:23:25 +08:00
Sign in to join this conversation.
No Label
No Milestone
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: M-Labs/thermostat#24
There is no content yet.