search

braincore / pid-rs

A PID controller for Rust projects.
Apache License 2.0
92 stars 26 forks source link

giving setpoint in constructor #2

Closed riaqn closed 5 years ago

riaqn commented 5 years ago

Currently it panics if we don't use update_setpoint. Maybe it's easier, and semantically clearer if we just pass it to the construcor, just like kp,ki,kd. We can still change it later if we want.

braincore commented 5 years ago

I'll have to check back to my original use case where I was creating a Pid before I knew the setpoint. But what your saying sounds reasonable. Maybe I'd still have the argument be an Option<> in case the caller intentionally does not want to have to set it to something bogus.

riaqn commented 5 years ago

In fact I think it shoud be plain <T> instead of Option<>. setpoint is an integral part of PID algorithm just like kp,ki,kd. Constructing a PID controller without setpoint doesn't make sense anyway, and introduces panic.

If the user doesn't want to set setpoint right away, they can just pass the default value appropriate for their context. For example, for a heater, they can pass -273 Celsious.

braincore commented 5 years ago

Yes, I agree. Looking back at the original project that had me create this, the need to define the setpoint later was idiosyncratic.

I'll get to changing it in a few days. Happy to accept a pull request as well.

Don't mean to pry, but mind sharing what you're building?

riaqn commented 5 years ago

I should have time tomorrow to make a PR.

I'm building a thermostat on stm32, to keep a cooler warm (around 40C).

braincore commented 5 years ago

Cool. Do you have any guides to recommend for embedded Rust? I haven't looked into the space much but I'm wondering what the tradeoffs are like.

riaqn commented 5 years ago

Rust on std32 is I would say very developed. For a quick look of the ecosystem see https://github.com/rust-embedded/awesome-embedded-rust

For a quick intro guide see https://rust-embedded.github.io/book/

The major tradeoff is you have to use no_std. So you dependencies have to be no_std as well, I guess that's a limitation. For heap allocations you can still use alloc-cortex-m, but I never tried.

riaqn commented 5 years ago

I just made PR #3

braincore commented 5 years ago

Accepted. Thank you for the PR and the embedded tips.

I'll publish a new version v2.0.0 soon.

braincore commented 5 years ago

New version v2.0.0 is published.