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

ADS1115 Update docs - for discussion #21638

Merged
merged 4 commits into from
Jun 8, 2023
Merged

ADS1115 Update docs - for discussion #21638

merged 4 commits into from
Jun 8, 2023

Conversation

hamishwillee
Copy link
Contributor

This updates the docs for the ADC1115 ADC in order to make it clear:

  • whether the driver is installed by default, and if not where.
  • how the driver is enabled and whether/where it is enabled by default.
  • if it is enabled, having a link to the parameter.
  • What the ADC1115 actually is, with a link to the manufacturer.

It is a test case for similar changes I hope to make across the docs, and to explore what might be automated. I'll add that discussion in a separate comment.

@hamishwillee
Copy link
Contributor Author

hamishwillee commented May 25, 2023

@bkueng I would appreciate your review of this, but more importantly, I'm wondering what we might practically do to allow this to be reused or used in user docs.

I think this is sufficient for module docs - if you're looking at the driver then you probably know what an ADC is, and why you want it, and how you find out details about its usage.

But in user docs you know a lot less, which is why we have the peripherals section - such as https://docs.px4.io/main/en/power_module/

Ideally in docs for this case I'd have a top level ADC page that explained

  • what an ADC is,
  • the fact that you need it if you have, say, an analog battery monitor and you want to connect it to a device that only has I2C digital output.
    • Are there any other common reasons?
  • List to variants we support
  • Any "common configuration" for all devices of the type - i.e. how they are wired.

The top page might link to device specific information, though usually if there is much I put that in its own page. That information includes:

  • What the item is
  • Manufacturer
  • How it is enabled - what the driver is, getting it in firmware, and turning it on.
  • Whether it needs to be enabled - ie what boards it is present in, and which it is enabled in or not.

What I'm after is a way to automate getting information about what we support as it arrives, and as it is dropped.
Where possible I'd like to auto generate list of information that are associated with the device such as "what boards include this in firmware", "What are its parameters", "What is the enable parameter, and is it enabled by default"?

How we might do this really depends on what can be inferred automatically and what needs to be specified. I mean it might be that we could get 99% of this by running a module_docs script that greps each module to get the params and the key for kconfig, uses conventions like _EN to get the name of the enabling parameter, and so on. We might still need to have some extra docs or formatted comments to get some info - e.g. @manufacturerurl, @purchaselinks

So I was thinking a script that ran across the docs and built a representation of all the information that you might need for user docs. Then use that to build user docs, or pass to the docs system to generate.
It would be better to do it all in PX, but then you'd need more in the source code such as the user-facing description, and so on.

What do you think? What fields would we need to have in source docs to be able to extract things like "enable params", and where would those live? Close to the driver docs ideally.

Imagine I'm doing the work as your minion :-)

@hamishwillee
Copy link
Contributor Author

hamishwillee commented May 25, 2023

@bkueng I've been thinking about this a little more. It might be useful to export a whole lot of data in our current parsers as json or similar for easy parsing, and add a few more parsers. We can then generate data from that data and repurpose as makes sense.

For example

  • For module parser:
    • include the parameters belonging to each module in the module docs.
    • include the kconfig key that includes the module in firmware.
  • Board parser that gets all the key settings (ie. what is in firmware)
  • Airframes parser - add parameters modified by airframes

With all that in json we could update module parser, for example to include the parameters, and also to make statements about board support like "Is in most boards by default, with exception of X, Y, Z, and is enabled using ABC params".

With this info on hand we could more easily dual-generate user-facing docs or tools and whatever other docs we want in future from pre-extracted information. Even if I can't do that, having a JSON that shows me the essential information about added/removed/changed data in a module-centric way would make automated tracking of change easier.

Don't know if you see that as worthwhile, or if there are better ways to achieve the same goals. It feels empowering though, and smart to separate data extraction and rendering.

@hamishwillee
Copy link
Contributor Author

Another idea, render the description in user docs as user docs about the associated hardware, not about the module?

@junwoo091400 junwoo091400 added Documentation 📑 Anything improving the documentation of the code / ecosystem Discussion (needed) 💬 Drivers 🔧 Sensors, Actuators, etc labels Jun 1, 2023
@bkueng
Copy link
Member

bkueng commented Jun 1, 2023

Makes sense. There's some overlap with device manifests, and we should think about how/if to use the same information for a device configuration UI in the GCS (similar to actuators).
And possibly further expand it to things like flight modes or ekf (metadata -> generate docs with config options).

@hamishwillee
Copy link
Contributor Author

Sounds good! I think you'd be best person to make specific design requirements on the extended possibilities.

Is this PR otherwise good to merge?

bkueng
bkueng previously approved these changes Jun 6, 2023
Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, can you run make format?

@hamishwillee
Copy link
Contributor Author

Done. Thanks!

@bkueng
Copy link
Member

bkueng commented Jun 7, 2023

There's still a failure:

Line too long (146 > 120) in drivers/adc/ads1115:
 It is enabled/disabled using the [ADC_ADS1115_EN](../advanced_config/parameter_reference.md#ADC_ADS1115_EN) parameter, and is disabled by default.
Error: validation failed

Maybe we don't need to be that strict with line lengths, I just added it to be consistent with the source code.

@hamishwillee
Copy link
Contributor Author

Maybe we don't need to be that strict with line lengths, I just added it to be consistent with the source code.

As you know, I prefer sentence breaks in our docs. But I think that as this is in source, we should have consistent rules. If this proves difficult we can revisit later.

Fixed for now. Thanks

@bkueng bkueng merged commit ea61d74 into main Jun 8, 2023
82 of 84 checks passed
@bkueng bkueng deleted the hamishwillee-patch-3 branch June 8, 2023 06:55
harrisondragoon pushed a commit to harrisondragoon/PX4-Autopilot that referenced this pull request Jun 30, 2023
antbre pushed a commit to BioMorphic-Intelligence-Lab/PX4-Autopilot that referenced this pull request Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion (needed) 💬 Documentation 📑 Anything improving the documentation of the code / ecosystem Drivers 🔧 Sensors, Actuators, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants