Multi-device support

[alexvlockwood-bes]

Hi! I'm Alexis Lockwood, a software engineer at Boulder Engineering Studio; I found this driver useful for a recent project with some admittedly heavy modifications, thought I'd drop by and try to contribute something back.

Changes I've made

• Remove #defines for display timing setup, instead allow a struct of values to be provided at runtime.
• Give all functions a context parameter eve_context_t *, with that struct containing all the necessary per-instance state as well as a collection of function pointers implementing the underlying lower-level routines.
• Add a timeout facility to the wait-while-busy loops.
• Probably broke DMA :wink:

Still planned

• Factor the function pointers out of the context struct, using a second level of indirection to allow this to be stored in flash rather than wasting all that RAM.
• Fix DMA. The breakage is only temporary as I definitely want DMA myself too.

This is definitely not ready for immediate upstreaming. I've definitely broken all the example code, and some of these changes will balloon the generated code for small targets like AVR and PIC.

Additionally needed to properly merge upstream

• Allow compile-time configuration on how the function pointer struct is accessed, allowing it to be e.g. PROGMEM on AVR.
• Allow the use of a single state context for builds not wanting to use multi-display support, avoiding passing around the useless pointer.
• Fix the examples.

When I've finished the "still planned" changes, I will certainly share with you the modified code — just the four files EVE.h, EVE_commands.h, EVE_commands.c, EVE_config.h — no matter what. Let me know if this sounds like something you'd be willing to merge, as I may be able to spend some time making the rest of the changes needed to get this upstreamed.

cc @austinbes

[RudolphRiedel]

Let me start with making clear that english is not my native language. :-) I am definately interested to see this.

Remove #defines for display timing setup, instead allow a struct of values to be provided at runtime.

Okay, but why? What is the benefit of writing somestruct.hsize over EVE_HSIZE? Why would anyone ever change these values at runtime?

Give all functions a context parameter eve_context_t *, with that struct containing all the necessary per-instance state as well as a collection of function pointers implementing the underlying lower-level routines.

Again, for what purpose?

Add a timeout facility to the wait-while-busy loops.

What wait-while-busy loops? The ones for the SPI transfers? How could these be failing at all? If the SPI is not working, there is a bigger problem and it fails right from the start.

Ah okay, so "Multi-device support" would be support to use more than one display? I am not really convinced that I want this.

On the one hand I am intrigued. On the other hand I never even thought about this and still do not see any potential in using more than one display. I would rather use a controller per display if I had a project that would need more than one display.

Like this:

I designed this for a project, it fits on the backside of a EVE2-70G. The controller is a ATSAMC21E18A-AU which is a Cortex-M0+ with 256k FLASH. It takes 7...30V and the interface is CAN-FD. We designed a tablet-like case around the EVE2-70G. This whole thing works as a control-unit now. The next revision of the board gets rid of the level-shifter by having the controller run at 3.3V. And with the BT8xx memory size became a non-issue.

If the customer would ask for a second screen to display additional information on I would just build annother unit with different software.

I am afraid that adding multi-display support will make everything even slower than it already is. Best case scenario for the majortiy of applications which use a single display would be code that is more complicated to understand than it already is now.

I could agree to a few things like putting the display paramters into a structure. But as a whole I see no benefit over one MicroContoller per display.

Maybe there is a benefit for very special aplications like forming a cube of six modules. Or having two displays back-to-back for some purpose. You would need a tight space in order to benefit from this at all.

But this is, well, very special and should not be allowed to have a negative impact on everything else.

Anyways, this is how I see it right now. :-)

[alexvlockwood-bes]

Alright, as promised the code as I have it is at https://github.com/alexvlockwood-bes/FT800-FT813/tree/4.0-bes-multiple. I see you're not particularly interested in this changeset so I'm not going to press further — not really worth arguing about the merits of supporting multiple displays from one master microcontroller or having timeouts in busy-loops. Thanks for the code, it works very smoothly and was a great starting point for our needs. :+1:

[RudolphRiedel]

Thank you for the feedback! I will check out what you did, it is not unlikely that I could learn a thing or two - I can only hope I will. :-)