Jump to content

Contributions / Pull requests


Steffen

Recommended Posts

Hi!

What is the current situation regarding pull requests for 2.8? Does it still make sense to open new PR's for 2.8 or will those not be accepted due to 3.0 being around the corner?
The 3.0 topic hasn't seen an update in a while so this is a little unclear. I do have a bunch of additions (i.e #26)  that i need for our product and would prefer to have those in upstream rather than maintaining local patches. However so far there hasn't been any activity there.

Link to comment
Share on other sites

V3 has been a much larger task than was originally planned and we have had less time than we expected to work on it. Ahh Gowing pains :)

Officially V2.8 is on lock down and we will only accept new drivers however there are some exceptions to that. The main reasons are that anything that gets added to the V2.8 branch needs to be ported to V3 which can significantly increase the work load for us. V3 has changed quite a few symbols to fix various V2 compile issues and some of the source structure and these always bite us in migrating fixes causing every change to have to be hand merged.

The exceptions are:

1. Bug fixes

2. Drivers

Having said all that - please submit your changes. Some we may decide to integrate anyway, some we may hold back. For that reason please submit them as one pull for each functional change rather than as one big one.

One thing we are very aware of is how long V3 is taking. If enough Delta's build up for V2 then we may even release a v2.9 just to keep things moving forward. Without the pull requests there though, there is no pressure to do that.

Link to comment
Share on other sites

Alright i'll put them up as i get around to it. As for 3.0, is the current state accessible somewhere? I assume you are using the version jump to break some of the api which might be a good opportunity to provide further feedback on some things that we'd like to see changed/improved, but which cannot be done without changing the api.

Link to comment
Share on other sites

At the moment the Dev list is already too long for V3 but a having a list may present some ideas that can be easily added as we go.

The major differences with V3 are:

1. Drivers will have a new internal API and will be built using gfxconf defines rather than being seperatly linked in. Existing drivers will all be rewritten as the new driver API is not compatible with V2 drivers.

2. All standard features (including drivers) of uGFX will be able to be built using the single file make method.

3. There will be a new GDISP API and many new capabilities. There are gfxconf defines (on by default) that support existing V2 code except for certain multiple display situations. The new API supports all displays on a single drawing canvas similar to how windows supports multiple displays except that uGFX also supports overlapping displays.

4. There are certain symbol changes to fix V2 compiling issues on some platforms eg RED becomes GFXRED, TRUE becomes GFXON or GTrue depending on context, numerous gfxconf changes. Again V2 compatibility is turned on by default.

In other words most existing V2 uGFX applications will recompile without changes while still allowing uGFX to move forward.

Post v3.0 we will look at changing GWIN to use an internal event model that will fix many GWIN shortcomings and improve extensibility.

Link to comment
Share on other sites

PS. In case you are wondering the new API's are not ready to share yet as they are still in flux.

Later on we may also call for assistance in converting existing drivers or module code to the new internal APIs. If anyone would like to volunteer (and get pre-release access) then please let us know. We will then contact you when we are ready.

Link to comment
Share on other sites

Heres a few things that come to mind as sort of a wishlist. Though some things may not apply to 3.0 if you are primarily focused on specific modules. For a few of those i might also make pull requests myself if we actually require them.

  • Add prefixes to all public types, defines and functions to avoid conflicts with other code. This specifically refers to things like `TRUE` which you already mentioned but also extends to many others like `justifyLeft`, `powerOn` or `color_t` of which some are quite likely to cause issues with other code.
  • I'd recommend having v2 compatibility turned off by default as it is only relevant for those upgrading the library. If it is on by default then people may mistakenly use it for new projects and run into the same old issues.
  • More use of `const` to allow for better optimization. Many of the apis (i.e `gwinCheckboxIsChecked`) don't use `const` despite only reading from the arguments. 
  • Widget options should use flags or variables on the widget object rather than custom drawing functions for things like text justification. This would seem more fitting and can be handled consistently across all widgets for common things like borders and text justification. The label widget already does this partially in its internal code. The custom drawing functions seem more like a dirty hack at the moment.
  • Add a toggle mode for the button widget, when used the button stays pressed and only is unpressed when used again. Currently the checkbox widget seems to be abused for the same functionality via a custom drawing function. This could be handled via a flag. This should come with new events for this mode (toggled on, toggled off).
  • Add a monospaced font (i know i can add my own, but its inconvenient)
  • Add more size options for the UI1 and UI2 fonts. Depending on the display the regular one might be too small while the larger one is already too large.
  • Add support for chinese and other characters to the included fonts. These can be disabled by default via an #ifdef but being able to just enable them on demand would be much more convenient over dealing with fonts and their licenses to get my own.
  • Some way to draw lines, boxes and circles inside containers/widgets would be neat. Using the gdisp/gwin functions does not really work for that as they will not remain on their own on a redraw. I suppose this could be done by having basic widgets for boxes, lines and circles.
  • A way to identify the widget type from a `GHandle`. This currently can be done with `gwinGetClassName` however comparing those strings isn't particularily efficient. A enum value would be much nicer to use.
  • Have a user settable pointer per widget similar to the tag but usable for any kind of data. This would allow quick access to user data from event callbacks without the need for additional lookup tables or such. This could probably replace the tag entirely. (Already suggested in another thread)
  • For inputs (keyboard, mouse, toggle, ...) provide api functions to trigger them from software for more flexibility. I.e similar to `gwinTextEditSendKey` but on driver level so that it works with any widget that takes keyboard events.
  • Interrupt driven mode for toggles, currently only polling seems to be possible despite our µC having nice interrupt notifications for each pin. With the functions from the previous point this could be handled easily, just would require some extra work to get out of the interrupt context.
Link to comment
Share on other sites

1. Already included in V3 at least for GDISP and his stuff.

2. For v3.0 V2 compatibility will be on, it will be turned off by default in a later 3.x release.

3. Consts are added where we find they should be. Note that the example given, if you are talking about the gwin parameter, it shouldn't be const as the gwin object may need to be thread locked which may require altering the lock on the gwin object. Just because the current implementation doesn't modify the structure, the API definition still needs to allow that it might. If you have specific examples where consts are missing (other than as legitimate API definition) please let us know.

4. There are code size implications for including all functionality and distinguishing it with flags. In some cases we have done that but as you say we have not been consistent largely due to slightly different methodology by different programmers. Some of these decisions will be reexamined when gwin is reviewed later. Custom draw routines however are very intentional. They provide a great way of custom extending drawing behaviour with little code and can be nested to provide "subclassing" style functionality.

5. The button should not be extended to have a toggle function as the button is designed to be momentary only. The checkbox is the state toggle widget. What confuses most GUI programmers from other platforms is the widget name which raises expectations of appearance. In uGFX the custom draw enables the presentation to be seperated from the functionality. There is no need to duplicate toggle functionality into the button widget thus saving much code.

6. Good idea.

7. Also a good idea but it is a lot of work as we hand designed UI1 and UI2.

8. Good idea. One disadvantage is that we currently build the font files from public font binaries (other than UI1 and UI2). That would no longer be possible if we add defines to control language inclusion. We would also need to pick a few common subset languages to add defines for.

9. This is difficult without either a method to store arbitrary object shapes and their drawing order (ram intensive) or support for overlapping windows. Again the gwin review will look at the end of those two approaches.

10. The gwin vmt address can be used for this purpose however these const values are not currently exposed publicly but can be derived imperically from existing objects eg at object creation.

11. Good idea

12. This will come with time.

13. Interrupt events are nice for responsiveness but are difficult at an os level due to restrictions on what can be done in them. They are also extremely non-portable, both at an operating system level and at a cpu level. Additionally there are also priority problems on some platforms. uGFX has therefore been designed to not require interrupt handling. It can however be used in some situations to improve responsiveness eg in gaudio and touch drivers. The toggle driver subsystem is very old (one of the oldest) and probably needs a review pass. This may or may not happen as part of the V3 driver changes but only if it is required for V3 toggle drivers to work (just due to time).

Link to comment
Share on other sites

  1. -
  2. -
  3. Good point on needing to be able to change them later. A better pick might be `gwinCheckboxCheck` where the bool parameter should be const'able without much hassle from what i can tell. Another might be `gwinGCheckboxCreate`, the pointers for `g` and `pInit` can have a const added (for the pointer, not the content. i.e GDisplay * const g) unless those are left changeable by design decision too.
  4. While i didn't try it i could imagine that all the custom drawing functions end up using more space than a switch() for the same purpose within the regular drawing function (in regards to something simple like text justification). Additionally a switch() could also be easily wrapped with an ifdef to completely remove it if the functionality is not needed. Of course the custom drawing functions do have their use and should not be removed. I just feel that text alignment and the like doesn't really belong there.
  5. Again i could imagine the custom drawing function to render the checkbox as button using more space than the few flag checks that would be needed in the button code. Its no major issue of course, using the checkbox works fine it just seems a little odd.
  6. -
  7. -
  8. Couldn't you build the public ones in each variant (language selections) and then just include the proper file depending on the defines?
  9. -
  10. The strings work for now and even their addresses could be used i suppose. Though that is a little hacky, having a proper api for it would be nifty.
  11. -
  12. -
  13. The api from 12. would do the job for me in this regard. Getting things out of the interrupt context and such can be left to the user while still offering the option to be able to use interrupts.
Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
×
×
  • Create New...