Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Passing refrences to Gpio pin port deifinitons to classes #275

Closed
W-M-D opened this issue Sep 6, 2019 · 33 comments
Closed

Passing refrences to Gpio pin port deifinitons to classes #275

W-M-D opened this issue Sep 6, 2019 · 33 comments

Comments

@W-M-D
Copy link

W-M-D commented Sep 6, 2019

Hello, I have an led array that has Rgb leds and i was wondering if I could pass a gpio to a function. It seems that gpios are set to a specific definition at compile time but there's no generic gpio class i can reference in my function definition? I'm not sure if this is the same as #201... Sorry...

@salkinium
Copy link
Member

You can pass the GPIOs function, for example GpioB0::set(bool level) to your function at runtime. Alternatively you can make your function a template function and pass the entire Gpio into it, but that just works for one function. Unfortunately a fully dynamic Gpio interface with base functions is too expensive, but maybe a computed interface might be interesting for modm.

@W-M-D
Copy link
Author

W-M-D commented Sep 6, 2019

That makes me really sad :/ why is it expensive if you pass by reference?
I was thinking of a class like this and then just passing a pointer of the port to it and having the class store the pointer. This would make it easy to reuse code. (this actually compiles lol)

class RGB_LED
{
public:

RGB_LED(Gpio * red_l,Gpio * green_l,Gpio * blue_l );

int8_t set_red();
int8_t set_green();
int8_t set_blue();
uint8_t led_handler();

private:

Gpio * red_led;
Gpio * green_led;
Gpio * green_led;

};

@salkinium
Copy link
Member

Then the Gpio class would need virtual functions, that each specific pin would implement. Those vtables add up to a lot of space, and are especially slow on AVRs. It also prevents efficient implementation of modm::platform::SoftwareGpioPort and other template/constexpr helpers. Consider the GPIO classes a universal building block for more dynamic things.

You can make RGB_LED a template class and pass all the GPIOs as template arguments, or create your own GPIO interface with virtual functions and wrap the modm GPIO classes into that.

@salkinium
Copy link
Member

For reference, this is the fancy stuff you can build with our GPIOs: #19 (comment)

@W-M-D
Copy link
Author

W-M-D commented Sep 6, 2019

Ah i didn't consider the AVR case... Thank you again for all that you have done with this project. It's very nice :D

@salkinium
Copy link
Member

salkinium commented Sep 6, 2019

For more reference, these are all platform specific GPIO functions that you'd need to implement with virtual functions. ~25 vtable entries at 4B each is 100B per GPIO class, thus for a LQFP100 package with maybe 80 GPIOs, that would be ~8kB just vtables. Although arguably you wouldn't use all 80 pins in your program, but just 10 GPIOs is 1kB vtables. There's probably more overhead though than just vtables.

A better way (and how it's usually done) is to encode the port/pin into an integer (uint8_t with high nibble for port, low nibble for pin) and then compute the register access at runtime. This works really well for STM32, since the GPIO registers are super regular, but on AVRs, the IO space is crammed full of random bits, and you need a lot of special cases. But I agree that we need a dynamic GPIO API going forward.

@W-M-D
Copy link
Author

W-M-D commented Sep 6, 2019

Yeah, that's how libopencm does it. But they don't join the port and pin into one variable so you have to send a port and a pin variable for each pin. Kind of annoying. I'm with you on the dynamic allocation as one of the strongpoints of C++ is the use of dynamic variables for code reuse. Even with the 8kB of overhead most modern processors have quite a bit of memory and storage. I think it might be worth it.

@salkinium
Copy link
Member

In that case, create a VirtualGpio class with all the functions you need, the use a template <class Gpio> class VirtualGpioWrapper : public VirtualGpio {} class that implements those functions using the template argument. Then you can have both worlds.

@W-M-D
Copy link
Author

W-M-D commented Sep 6, 2019

I'm not quite sure I follow. But I'll try!

@W-M-D
Copy link
Author

W-M-D commented Sep 6, 2019

Also i just realized how this project was built using lbuild (i didn't know what it was before but having used jekyl i understand its power). That's sexy af. I'm used to hand rolling C++ because of the dynamic aspect. Maybe that would be the way to go. I'm so lost haha.

@salkinium
Copy link
Member

salkinium commented Sep 6, 2019

@W-M-D
Copy link
Author

W-M-D commented Sep 6, 2019

Damn, I must be really dumb 😆 . I kind of understand what you're talking about now. Just seems like a really complicated way of doing something that should be simple :/ . Thank you again for your help :D

@salkinium
Copy link
Member

salkinium commented Sep 6, 2019

This stuff is difficult to learn, it took me a decade to get to this point. You're not dumb!

Just seems like a really complicated way of doing something that should be simple :/

Unfortunately this is on purpose, modm wasn't designed for a dynamic runtime, instead moving a lot of things into compile time or even code gen time (in Python via lbuild).

This makes it much easier to use on very low resource devices (we started on AVRs and still fully support them), but obviously much more difficult for other use-cases.

I'm very much aware of the limitations of this method, and am actively working on a better balance between an "all template HAL" and an "all runtime HAL". My initial explorations show a really good compromise between the two.

@W-M-D
Copy link
Author

W-M-D commented Sep 6, 2019

Ah but I have been doing this for much longer than 10 years 😆 . Yeah, there is a huge benefit to statically compiled code. I'm actually thinking about using lbuild to make some generic devices (RGB led, Motor, buttons etc) . But i think it would take too long to get into it. I don't have much time to do this.

@W-M-D
Copy link
Author

W-M-D commented Sep 6, 2019

Also that gist is missing the hardware.hpp. I'm pretty sure it's intentional. I'm still figuring it out 😆 .

@salkinium
Copy link
Member

hardware.hpp just contains the declarations of the functions and objects. The files are from a generic CAN-to-GPIO expander, with 24 ports of 2 GPIOs each (so 48 GPIOs per CAN node). It's meant to be flexible and very fast and virtual functions have a fixed overhead (two address resolutions + function code), a runtime computation of the required registers accesses does not.

I just wanted to show you that you can then use the base class elva::VirtualGpio as the type for an array of pointers to the specific GPIO implementations. That's what I meant with using the VirtualGpioWrapper to implement the specific GPIO.

@W-M-D
Copy link
Author

W-M-D commented Sep 6, 2019

Yeah, I know. I tried declaring everything but I'm still getting type errors. I guess I just don't understand templates well enough. reverts back to old project quietly weeps

@salkinium
Copy link
Member

I'm sorry, modm is very research-y… hard to compete with all the other libs out there 😢.

reverts back to old project quietly weeps

While My Guitar Brain Gently Weeps
https://www.youtube.com/watch?v=8hUOKjy-9-o

@W-M-D
Copy link
Author

W-M-D commented Sep 6, 2019

No it's a very good library. For sure better than anything else out there. But my coding style depends so heavily on dynamic code reuse that it doesn't really work for me. Surely better for smaller projects. Everything works as needed.

@W-M-D
Copy link
Author

W-M-D commented Sep 6, 2019

i just don't understand. hardware_io.cpp:111:9: error: 'GpioControl' in namespace 'elva' does not name a type. What object type is GpioControl supposed to be. I'm lost i really feel bad bothering you on this. Do you have a btc address ?

@salkinium
Copy link
Member

I yanked that very quickly from the project into a Gist, it was meant more as an inspiration.

But here's a compiling and tested example on my modm fork, see the feature/virtual_gpio branch: https://github.com/salkinium/modm/tree/feature/virtual_gpio/examples/nucleo_f411re/virtual_gpio

@salkinium
Copy link
Member

The VirtualGpio class should work for all STM32, except maybe the F1 series, since that has a mildly different hardware implementation. What device are you using?

@W-M-D
Copy link
Author

W-M-D commented Sep 6, 2019

I'm using an stm32f071v8. Thanks, I was inspired, just lost.

@W-M-D
Copy link
Author

W-M-D commented Sep 6, 2019

That works :D Thank you so very much!

@W-M-D
Copy link
Author

W-M-D commented Sep 7, 2019

I just realized this is even worse for the Timers haha. Guess I'll have to do a massive switch case or something...

@salkinium
Copy link
Member

I mean, modm really goes out of its way to NOT use C++ polymorphism with virtual interfaces, so you're going to encounter these issues over and over again.

@salkinium
Copy link
Member

I'm closing this issue since I think the original issue has been resolved. You can of course still continue commenting!

@W-M-D
Copy link
Author

W-M-D commented Sep 12, 2019

@salkinium not related but I'm trying to understand how to pass timer channels to different timer inputs. I have a channel 3 pin in that i want to pass to channel 1 for the encoder. looking at the timer diagram there seems to be a link.

@salkinium
Copy link
Member

We don't have an API for that, you need to set the registers directly. It's on my radar, but requires a lot of data (so has a dependency on modm-devices).

@W-M-D
Copy link
Author

W-M-D commented Sep 12, 2019

Ah understood :) thanks I just changed it in hardware for now lol

@rleh
Copy link
Member

rleh commented Sep 13, 2019

Implementing a generic API for timers is probably not possible and in my opinion the wrong attempt of abstraction. On STM32 every timer and every channel sometimes has different features/options and behaves slightly different.
If you look at more manufacturers, it gets even worse in terms of compatibility/interoperability.

I could imagine a platform independent API which provides a PWM signal with adjustable frequency and duty cycle at one pin.

@W-M-D
Copy link
Author

W-M-D commented Feb 25, 2020

@rleh yeah you're actually right, It was still a better option since i was able to get two identical timers for my operations.

@W-M-D
Copy link
Author

W-M-D commented Feb 26, 2020

Can't wait for RISC-V to come and fix everything lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants