Allow non-float types

[usbalbin]

Relax trait bounds to allow types other than float

This allows using types like i8, i32 etc. or any other user type that satisfies Debug + PartialOrd + num_traits::Signed + Copy

[usbalbin]

Motivation:

I am considering using this in a microcontroller where integers would be preferred over float due to hardware and performance limitations. This change enables me to pick any (signed) numeric type i could possibly want.

Note

When using integers or similar, make sure to pick a large enough type as to avoid overflow in any of the multiplications in the next_control_output function.

[Owez]

Thanks for the PR, this is a really good feature to have. A few minor changes and I think it's ready to go

[Owez]

When using integers or similar, make sure to pick a large enough type as to avoid overflow in any of the multiplications in the next_control_output function.

Ideally having tiny types which will cause UB will give a compiler error, but if not it needs to be documented somewhere eventually. Would one of the overflowing_..-ish methods be good here to clamp to the max value when calculating if compiler errors aren't feasible?

[usbalbin]

num_traits::Saturating and num_traits::SaturatingMul does seem like they would be nice for that, however that is only implemented for integers. Not sure if there is any way to work around that?

[usbalbin]

I guess we could define Number something like

trait Number: PartialOrd + num_traits::Signed + Copy {
    fn safe_mul(self, other: Self) -> Self;
    fn safe_add(self, other: Self) -> Self;
}

and only implement Number for the built in types (using saturating_mul/add where relevant), then it would be up to the user to implement it for their types.

However this would make it harder to use 3rd party types

[usbalbin]

I guess something similar could be done for apply_limit to avoid the requirement on num_traits::Signed. Currently in this PR we need to be able to negate limit to get the lower limit, however for unsigned values this should probably always be 0

[Owez]

num_traits::Saturating and num_traits::SaturatingMul does seem like they would be nice for that, however that is only implemented for integers. Not sure if there is any way to work around that?

Theres OverflowingMul which looks to be implemented for for all types which can be used as a workaround, though having an if its_return.1 { /* set its_return.0 to MAX */ } everytime its ran might impact performance too much for a change meant to fix what shouldn't be broken :)

I'll add some docs about it in the controller somewhere for small types

[Owez]

I guess something similar could be done for apply_limit to avoid the requirement on num_traits::Signed. Currently in this PR we need to be able to negate limit to get the lower limit, however for unsigned values this should probably always be 0

I've implemented a version of this locally but I'm not sure what to do with the d_unbounded derivative kick because it needs negation with -value. Here's the work-in-progress I've got so far (apply_limit is now num_traits::clamp(value, T::neg_or_zero(limit), limit)):

pub trait Number: PartialOrd + num_traits::Num + Copy {
    /// Makes the value negative (`-value`) if signed, otherwise leaves it as `0` 
    fn neg_or_zero(val: Self) ->  Self { Self::zero() }
}

impl Number for f64 {  fn neg_or_zero(val: Self) -> Self { -val } }
impl Number for f32  {  fn neg_or_zero(val: Self) -> Self { -val } }
impl Number for i128  {  fn neg_or_zero(val: Self) -> Self { -val } }
impl Number for i64  {  fn neg_or_zero(val: Self) -> Self { -val } }
impl Number for i32  { fn neg_or_zero(val: Self) -> Self { -val } }
impl Number for i16  { fn neg_or_zero(val: Self) -> Self { -val } }
impl Number for i8  {  fn neg_or_zero(val: Self) -> Self { -val }}
impl Number for isize  {  fn neg_or_zero(val: Self) -> Self { -val } }
impl Number for u128  {  }
impl Number for u64  {  }
impl Number for u32  { }
impl Number for u16  { }
impl Number for u8  { }
impl Number for usize  {  }

[usbalbin]

[..] but I'm not sure what to do with the d_unbounded [...]

Oh, right... I guess that would sort of need to be signed?

[Owez]

Oh, right... I guess that would sort of need to be signed?

Googled quickly and looks like it can be done: https://ledin.com/integer-algorithms-implementation-and-issues/
Best leaving this to a seperate PR to figure it out; its possible but it might need a different formula

[Owez]

Looks like isize doesn't work so I've marked it down in the docs. Will merge now, I don't think this is actually a breaking change because it's loosened the bounds so this might go in as a 4.1.0 in a while.

[Owez]

@braincore I've squashed here following https://medium.com/singlestone/a-git-workflow-using-rebase-1b1210de83e5. Want me to do a plain merge or rebase again onto master?

[braincore]

@Owez Looks correct. I've rebased so far as I prefer the linear history so let's keep doing that.

@usbalbin Solid contribution. Just wondering, which microcontroller are you using and how smooth has the embedded rust experience been?

[usbalbin]

I am using an ESP32 with esp-idf-hal. Quite nice actually, I really like the concept of having peripherals being objects ties into rusts ownership system. Making it impossible to use the same hardware resources for different things by accident.

Other than that there have been some surprises, in the esp-idf-hal case, things like floats. There is hardware support for float in the mcu, however you can not use them from interrupts since those registers are not saved. That crash did take quite some time to figure out(I should have read the doc more closely :) ). I guess that is something which would be nice if the compiler could somehow magically tell me about... There is also the story about doc, at least as of right now esp-idf-hal is not that well documented so you have to read the corresponding C doc and/or try to find some example to know what to do. However I have been met by a great community that is very happy to answer my esp related questions and help me out.

Anyways compared to C, it's great. Not having to turn your entire codbase upside down just because you forgot to put volatile in front of your shared variable or that you somehow happened to use the same timer when you reused a code snippet from another project, is awesome. Also in the esp-idf-hal case we get access to std so we don't have to re-implement every little simple thing from scratch(and less simple things). Among other things we get access to threads, println! and the fmt machinery. Crates like serde and pid(outside interrupts) just works. For more light weight frameworks like esp-hal (not idf) which does not have std, async/await is used instead of threads which does sound quite nice(have not tested it myself yet though).

So in summary I think there are some great things and some less then great things. However overall the good does, in my mind, overweight the less good. I believe most of the bad things are planned/being worked on.

[braincore]

@usbalbin Thanks for the rundown. I published an assortment of hardware peripheral crates for rust, but they were always used on a Raspberry Pi via i2c since the embedded story was rough at the time. It's great to hear that it's workable these days.

Checking whether the FPU functions inside an interrupt is not exactly something that would have been on the top of my list either :).