Simplify 8-bit display interface#16
Conversation
|
Any feedback on this one? Concerns? |
|
Sorry, didn't get to testing it yet. No concerns from just reading the code. |
|
This only works if all of the data pins are the same type, which is not always the case. For example, in my application With stm32f1xx_hal you can work around this by "downgrading" pins to a single type, but I am not sure if all HALs support that. |
|
Yeah, it does require downgrading pins. |
|
#20 solves this problem in a different way, feel free to check it out and let me know what you think |
|
@andresovela I have to agree with @GrantM11235 here that having to downgrade pins is not ideal. The main problem that I see is that using downgraded pins usually cause some additional overhead due to the dynamic calculation of the right addresses and bits and that is not ideal for this usecase. |
|
Alright. Closing this PR in favor of #20 then. May I ask about the "additional overhead" though? I was not aware of that. What dynamic calculation of addresses and bits? |
|
Here is a simplified example of how impl PA0 {
fn set_high(&mut self) {
port_a_registers.set_output_bits(0b00000001);
}
}
impl Pin {
fn set_high(&mut self) {
let registers = match self.port {
Port::A => port_a_registers,
Port::B => port_b_registers,
Port::C => port_c_registers,
Port::D => port_d_registers,
};
let bits = 1 << self.pin;
registers.set_output_bits(bits);
}
}To be honest, I don't actually think that performance is a big concern here. With inlining and LTO, |
Usual pins are zero-sized types, the implementation of which get monomorphized into pretty much ideal code. OTOH downgraded pins are by concept unified structures containing registerblock adresses and pin offsets so a generic implementation can be used.
Well, it is theoretically possible that it compiles into the comparable code but it's somewhat unlikely due to dependence on a number of factors (implementation, optimization flags and what not)... |
I simplified the API for the parallel GPIO interface by grouping the bus into an array of pins rather than 8 individual pins. This allows to further simplify the
set_value()function.