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

doc: Edit of manual reference pages #2466

Merged
merged 8 commits into from
Jul 18, 2024

Conversation

architeuthidae
Copy link
Collaborator

@architeuthidae architeuthidae commented Jun 28, 2024

ARTIQ Pull Request

Added missing frontend methods to Utilities (creating get_argparser when necessary). Refactored detailed explanation of artiq_coremgmt out of reference to keep it clean and into Core device (cut down some). Added (complete?) list of currently used config keys. Formatting fixes in reference pages and somewhat tangentially in Installing (not strictly related except that the config key list is pretty heavily interreferenced with it.)

Added AFWS password restrictions as referenced from support instructions

Description of Changes

Related Issue

Closes #1025

Type of Changes

Type
📜 Docs

Steps (Choose relevant, delete irrelevant before submitting)

All Pull Requests

  • Use correct spelling and grammar.

Documentation Changes

  • Check, test, and update the documentation in doc/. Build documentation (cd doc/manual/; make html) to ensure no errors.

Git Logistics

  • Split your contribution into logically separate changes (git rebase --interactive). Merge/squash/fixup commits that just fix or amend previous commits. Remove unintended changes & cleanup. See tutorial.
  • Write short & meaningful commit messages. Review each commit for messages (git show). Format:
    topic: description. < 50 characters total.
    
    Longer description. < 70 characters per line
    

Licensing

See copyright & licensing for more info.
ARTIQ files that do not contain a license header are copyrighted by M-Labs Limited and are licensed under LGPLv3+.

@architeuthidae architeuthidae marked this pull request as draft July 2, 2024 02:39
@architeuthidae architeuthidae marked this pull request as ready for review July 2, 2024 03:05
@architeuthidae
Copy link
Collaborator Author

rebased for merge conflict

doc/manual/core_device.rst Outdated Show resolved Hide resolved
doc/manual/core_device.rst Outdated Show resolved Hide resolved


async def main_async():
args = get_argparser().parse_args()
Copy link
Member

Choose a reason for hiding this comment

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

Those AFWS coding style changes should not be in this PR, especially since they cause problems with the release-8 backport where afws_client does not use asyncio.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Minimized changes as much as possible but it's still necessary to create a get_argparser to render documentation in the manual, so the backport may still be inconvenient...? Not entirely sure what best to do about that

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed that these changes in afws_client should not be included in this PR for backporting purposes. I can create a separate PR for get_argparser for both the beta and stable branches.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay, reset afws_client entirely; Sphinx raises a warning about a missing argparser but it does still compile the page. Thanks and sorry for the trouble!

Copy link
Contributor

Choose a reason for hiding this comment

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

Already submitted a PR for stable: #2488 and beta on afws (Gitea). Hopefully, those warnings will be gone once they are merged.

doc/manual/utilities.rst Outdated Show resolved Hide resolved
doc/manual/utilities.rst Outdated Show resolved Hide resolved

* For Kasli or KC705:

If the ``ip`` config field is not set or set to ``use_dhcp``, the device will attempt to obtain an IP address and default gateway using DHCP. If a static IP address is nonetheless wanted, it can be flashed directly (OpenOCD must be installed and configured, as above), along with, as necessary, default gateway, IPv6, and/or MAC address: ::
If the ``ip`` config field is not set or set to ``use_dhcp``, the device will attempt to obtain an IP address and default gateway using DHCP. If a static IP address is nonetheless wanted, it can be flashed directly (OpenOCD must be installed and configured, as above), along with, as necessary, default gateway, IPv6, and/or MAC address: ::
Copy link
Collaborator

Choose a reason for hiding this comment

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

artiq-zynq doesn't use a default gateway, but I think you don't have to clarify that here...

artiq/frontend/afws_client.py Outdated Show resolved Hide resolved
artiq/frontend/artiq_ddb_template.py Show resolved Hide resolved
artiq/frontend/artiq_ddb_template.py Show resolved Hide resolved
doc/manual/core_device.rst Outdated Show resolved Hide resolved
@architeuthidae
Copy link
Collaborator Author

Re: last commit -- source order seems like a better choice; it e.g. groups all the decorators and all the argument types together, whereas alphabetical has them spread out in an unintuitive way. In theory it's nice to be able to find things alphabetically too but these references aren't that long and in practice I'm sure anybody would be using ctrl+F.

@architeuthidae
Copy link
Collaborator Author

Rebased (removed the formatting corrections to Installing, which were tangential anyway) to avoid a merge conflict with a future PR + cleanup commit history while I'm at it


To use this tool, it is necessary to specify the IP address your core device can be contacted at. If no option is used, the utility will assume there is a file named ``device_db.py`` in the current directory containing the device database; otherwise, a device database file can be provided with ``--device-db`` or an address directly with ``--device`` (see also below).

To read core device logs::

$ artiq_coremgmt log
Copy link
Member

Choose a reason for hiding this comment

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

At least this one is quite useful and should not be removed from the examples.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The command is already mentioned in both 'Installing' and 'RTIO', and the same information is provided by the auto-generated documentation a few lines below this. (Separately, it might be nice to have a dedicated section on the various logs and their management in 'Core device', which could go into more depth, but that'd be a different PR). If we prefer I can add it back, but none of the other utilities commands have their examples or guides on the reference page, so it seems a little out of place.

@sbourdeauducq sbourdeauducq merged commit 499eb42 into m-labs:master Jul 18, 2024
sbourdeauducq pushed a commit that referenced this pull request Jul 18, 2024
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.

Document net_trace and its performance implications
5 participants