Skip to content

Documentation review: fix outdated references, remove redundancy#39

Closed
Copilot wants to merge 18 commits intogroup-coefrom
copilot/update-documentation-and-cleanup
Closed

Documentation review: fix outdated references, remove redundancy#39
Copilot wants to merge 18 commits intogroup-coefrom
copilot/update-documentation-and-cleanup

Conversation

Copy link
Contributor

Copilot AI commented Feb 5, 2026

Documentation had drifted from the codebase: referenced modules that no longer exist, described GUI elements that don't exist, and contained redundant/completed content.

Changes

Removed redundant docs

  • terminal-definitions.md (duplicated by terminal-yaml-definitions.md)
  • simulator-symbol-alignment.md (completed)
  • Moved api-decoupling.mddecisions/0003-api-decoupling-analysis.md

Fixed outdated code references

  • xml_parser.py, xml_cache.py, app.pyxml/ subpackage, ui_app.py
  • catio_hardware.pycatio_dynamic_*.py modules

Updated for dynamic controller generation

  • Replaced "Hardware-Specific Controllers" with "Dynamic Terminal Controllers"
  • Documented get_terminal_controller_class() and YAML-driven controller creation

Fixed terminal-yaml-definitions.md

  • Single terminal_types.yaml (not multiple category files)
  • Added composite_types section, selected field, CoE subindices structure

Removed embedded code

  • Replaced all literalinclude directives with descriptions and source links per AGENTS.md

Fixed Mermaid diagrams

  • Added mermaid-simple CSS class for simple diagrams to prevent oversized fonts

Instructions to reviewer on how to test:

  1. Build docs and verify referenced files exist (e.g., src/catio_terminals/xml/parser.py)
  2. Compare docs/explanations/terminal-yaml-definitions.md against src/catio_terminals/terminals/terminal_types.yaml
  3. Verify Mermaid diagrams render at reasonable font sizes

Checks for reviewer

  • Would the PR title make sense to a user on a set of release notes

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

make boolean attributes look right!
refactor dynamic code into smaller modules
Copilot AI changed the title [WIP] Update documentation to match current code state Clean up and consolidate documentation Feb 5, 2026
Copilot AI requested a review from gilesknap February 5, 2026 09:13
@gilesknap
Copy link
Contributor

I don't think a deep review of the code has been applied to the documentation.

For example the page how-to/use-terminal-editor mentions xml_parser.py which no longer exists.

Take a detailed look at the code and update any such references.

Copilot AI changed the title Clean up and consolidate documentation Fix outdated code references in documentation Feb 5, 2026
Copilot AI changed the title Fix outdated code references in documentation Documentation review: fix outdated references, remove redundancy, update GUI docs Feb 5, 2026
Copilot AI changed the title Documentation review: fix outdated references, remove redundancy, update GUI docs Documentation review: fix outdated references, remove redundancy Feb 5, 2026
@gilesknap gilesknap marked this pull request as ready for review February 5, 2026 10:09
Copy link
Contributor

@gilesknap gilesknap left a comment

Choose a reason for hiding this comment

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

The Runtime Data Flow mermaid diagram has HUGE fonts, it looks silly and takes up too much space. Make sure you have reviewed how diagrams look as per the original prompt

Copy link
Contributor Author

Copilot AI commented Feb 5, 2026

I don't think a deep review of the code has been applied to the documentation.

For example the page how-to/use-terminal-editor mentions xml_parser.py which no longer exists.
...

Fixed in commit 1bd3c6a. The Architecture section in use-terminal-editor.md now correctly references xml/ subpackage with parser.py, pdo.py, pdo_groups.py, coe.py, catalog.py, cache.py instead of the old flat module names.

The Runtime Data Flow mermaid diagram has HUGE fonts, it looks silly and takes up too much space. Make sure you have reviewed how diagrams look as per the original prompt

Fixed in 558ee47. Removed the explicit %%{init: ...fontSize: '14px'...}%% directives from all Mermaid diagrams. They now use Mermaid's default font sizing which should render at a reasonable size.

gilesknap and others added 6 commits February 5, 2026 10:32
- Remove completed plan (simulator-symbol-alignment.md)
- Remove redundant terminal-definitions.md (content covered in terminal-yaml-definitions.md)
- Move api-decoupling.md to decisions/0003-api-decoupling-analysis.md
- Update README.md with actual project description
- Update todo.md to be more concise
- Add font sizing to all Mermaid diagrams
- Create documentation-plan.md with todos and agent prompts
- Fix all broken cross-references

Co-authored-by: gilesknap <964827+gilesknap@users.noreply.github.com>
Co-authored-by: gilesknap <964827+gilesknap@users.noreply.github.com>
- Update use-terminal-editor.md Architecture section with correct module names
- Fix xml_parser.py reference in beckhoff-xml-format.md to xml/parser.py
- Fix xml_pdo_groups.py reference in dynamic-pdos.md to xml/pdo_groups.py
- Fix AGENTS.md references to xml parsing modules

Co-authored-by: gilesknap <964827+gilesknap@users.noreply.github.com>
- Fix "Adding a Terminal" section: search is live/automatic (no Search button)
- Document filtering by type dropdown functionality
- Document "Add All" button for bulk terminal addition
- Improve troubleshooting section with correct cache update command

Co-authored-by: gilesknap <964827+gilesknap@users.noreply.github.com>
gilesknap and others added 2 commits February 5, 2026 10:48
- Replaced "Hardware-Specific Controllers" section with "Dynamic Terminal Controllers"
- Removed references to catio_hardware.py and explicit controller classes (EL10xxController, etc.)
- Added documentation for get_terminal_controller_class() factory function
- Documented the catio_dynamic_*.py modules and their roles
- Added link to terminal-yaml-definitions.md for YAML format details

Co-authored-by: gilesknap <964827+gilesknap@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 5, 2026

fascs-epics-ioc contains a reference to Hardware-Specific Controllers, these are no longer used and the equivalents are dynamically created by the modules catio_dynamic_*.py. change the doc to reflect...

Fixed in 5640ede. Replaced "Hardware-Specific Controllers" section with "Dynamic Terminal Controllers":

  • Removed references to catio_hardware.py and explicit controller classes (EL10xxController, etc.)
  • Documented how get_terminal_controller_class() creates controllers from YAML definitions
  • Added table of catio_dynamic_*.py modules and their roles
  • Added link to terminal-yaml-definitions.md for YAML format details

Copy link
Contributor

@gilesknap gilesknap left a comment

Choose a reason for hiding this comment

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

Runtime Data Flow mermaid diagram still has huge fonts. This is due to the global css you supplied, I believe. Please override that for simple diagrams like this that do not merit fitting to the page width

Copy link
Contributor

@gilesknap gilesknap left a comment

Choose a reason for hiding this comment

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

under The Base Controller heading there is embedded code.

obey the agents.md and never embed code. instead use a brief description and refer to the python module.

…class

- Added mermaid-simple CSS class that uses natural diagram size instead of 100% width
- Wrapped Runtime Data Flow diagram in mermaid-simple div to prevent oversized fonts
- Complex diagrams still use full width, simple diagrams use natural size

Co-authored-by: gilesknap <964827+gilesknap@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 5, 2026

Runtime Data Flow mermaid diagram still has huge fonts. This is due to the global css you supplied, I believe. Please override that for simple diagrams like this that do not merit fitting to the page ...

Fixed in 375143f. Added a mermaid-simple CSS class that overrides the full-width behavior for simple diagrams. The Runtime Data Flow diagram is now wrapped in this class so it uses its natural size instead of expanding to 100% width. Complex diagrams (like the High-Level Architecture) still use full width.

- Replaced literalinclude directives with brief descriptions and links to source files
- Updated fastcs-epics-ioc.md: CATioController, CATioControllerAttributeIO, MockADSServer
- Updated ads-client.md: RemoteRoute, AsyncioADSClient, AdsSymbol, MockADSServer
- Follows AGENTS.md guideline to never embed code from the repository

Co-authored-by: gilesknap <964827+gilesknap@users.noreply.github.com>
Copy link
Contributor

@gilesknap gilesknap left a comment

Choose a reason for hiding this comment

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

explanations/ads-client.html also includes lots of embedded code

Copy link
Contributor Author

Copilot AI commented Feb 5, 2026

under The Base Controller heading there is embedded code.

obey the agents.md and never embed code. instead use a brief description and refer to the python module.

Fixed in 513b3f5. Replaced all literalinclude directives with brief descriptions and links to source files:

  • fastcs-epics-ioc.md: CATioController, CATioControllerAttributeIO, MockADSServer
  • ads-client.md: RemoteRoute, AsyncioADSClient, AdsSymbol, MockADSServer

Copy link
Contributor Author

Copilot AI commented Feb 5, 2026

explanations/ads-client.html also includes lots of embedded code

Already fixed in 513b3f5. The ads-client.md file no longer contains any literalinclude directives or embedded code. All code references have been replaced with brief descriptions and links to the source files. You may need to rebuild the docs to see the updated HTML.

@gilesknap
Copy link
Contributor

closed in favor of #40

@gilesknap gilesknap closed this Feb 5, 2026
@gilesknap gilesknap deleted the copilot/update-documentation-and-cleanup branch February 5, 2026 14:01
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.

2 participants