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

Pimoroni TrackBall breakout description #56

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Gadgetoid
Copy link

I've raised this draft PR for the purpose of illustrating a real-world use of the yaml definition files. My intent is to - hopefully - illustrate some of the pain points with generating a definition and, subsequently, usable code.

A couple of points to discuss and perhaps raise into issues/PRs are:

  • It would be nice to have a bit property for functions for cases where bitStart == bitEnd
  • It could be useful to group registers into logical sets with an overarching description and/or defaults. IE: the LEDs in this definition - red, green, blue and white - all share behaviour in common.
  • Continuous integration could build a test definition into target languages and lint the result (flake8 for Python, etc)

Playing off the point above, a register group might appear thus:

registers:
        - LEDs:
            startAddress: 0x00
            length: 8
            title: TrackBall Red, Green, Blue and White LEDs
            description: Each LED is dimmable, with 256 brightness levels from 0-255 (off-full).
            - Red:
            - Green:
            - Blue:
            - White:

Admittedly this has more practical implications for the clarity and presentation of documentation than it does for code.

@Fleker
Copy link
Collaborator

Fleker commented Apr 10, 2019

In your LED case, each LED is a separate register?

Our CI process currently does generate code based on the example peripherals and checks against our cached versions and runs lint on the output.

@Gadgetoid
Copy link
Author

Yes- each LED is a separate register of width 8 bits, starting at offset 0x00. So Red would be 0x00, Green 0x01, etc. Another downside of this idea is that it's less explicit about addresses, but this is also an upside if- for example- you take a whole group of registers out of one definition and insert it into another, even if its address offset is different. (This may happen across IC families, although in practise I haven't encountered it much)

@Fleker
Copy link
Collaborator

Fleker commented Apr 11, 2019

If there was a good example of this, it'll be good to consider. I'm not sure if there's too much trouble in just keeping the four registers in close proximity to one another without having to logically group them.

It could be useful to create a separate top-level section for groups of registers, which in the code-gen may result in a function with multiple parameters.

set_leds(red = 0x90, green = 0x85, blue = 0x77, white = 0x20)

@Fleker
Copy link
Collaborator

Fleker commented Apr 14, 2019

What if we defined registers in the current way:

registers:
  - Red:
      address: 0x00
      length: 8
      title: Red LED
      description: Red LED saturation
  - Blue: ...

And then created a separate top-level:

groups:
  LED:
    - '#/registers/Red'
    - '#/registers/Blue'
    - '#/registers/Green'
    - '#/registers/White'

What do you think @chrisfrederickson @Gadgetoid?

@chrisfrederickson
Copy link
Contributor

@Fleker For inspiration, OpenAPI uses tags to create logical groupings.

https://swagger.io/docs/specification/grouping-operations-with-tags/

Following their implementation we could alternatively implement grouping and shared behavior the following way:

registers:
  - Red:
      address: 0x00
      length: 8
      title: Red LED
      description: Red LED saturation
      tags:
         - LED
  - Blue: ...

And then use $ref to reuse psuedoyaml behavior across different registers.

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

Successfully merging this pull request may close these issues.

3 participants