Onboard commit - #56
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an initial “onboard” documentation set for the XR Voice SDK, covering architecture, threading, configuration, component analyses, and OpenSpec workflow prompts/skills.
Changes:
- Introduces a comprehensive documentation suite under
docs/(architecture, APIs, XRAudio/XRSR deep-dives, validation summaries). - Adds OpenSpec skill definitions and prompts under
.github/skills/and.github/prompts/. - Provides developer-facing quick-start and cross-reference navigation docs.
Reviewed changes
Copilot reviewed 43 out of 87 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/XRSR_Advanced_Features_Analysis.md | New XRSR advanced-features writeup (perf, monitoring, config, security). |
| docs/XRAudio_Real_Time_Processing.md | New real-time audio processing pipeline overview. |
| docs/XRAudio_Configuration_Management.md | New config/JSON parsing and plugin config documentation. |
| docs/XRAudio_Component_Analysis.md | New architecture analysis for XRAudio component. |
| docs/XRAudio_Atomic_Operations_Threading.md | New atomics + thread safety documentation for XRAudio. |
| docs/Versioning_System.md | New SDK/component versioning documentation with build-time generation. |
| docs/Validation_Quality_Assurance_Completion_Summary.md | New validation/QA completion summary for the doc set. |
| docs/Threading_Model.md | New end-to-end SDK threading model documentation. |
| docs/SDK_Architecture.md | New high-level SDK architecture doc. |
| docs/README.md | New documentation index / navigation entry point. |
| docs/Quick_Start_Developer_Guide.md | New quick-start integration guide with build + platform examples. |
| docs/Known_Limitations_Investigation_Areas.md | New known limitations + investigation roadmap document. |
| docs/Cross_Reference_Navigation_System.md | New cross-reference map linking docs ↔ code ↔ config. |
| docs/Cross_Component_Integration_Analysis.md | New cross-component integration analysis (XRAudio/XRSR/XRSV). |
| docs/Component_Dependencies.md | New component boundary + dependency analysis doc. |
| docs/Build_System_Configuration.md | New build/CMake configuration documentation. |
| docs/API_Interface_Documentation.md | New consolidated public API documentation. |
| docs/API_Cross_Reference_Validation_Report.md | New API cross-reference validation report. |
| .github/skills/openspec-propose/SKILL.md | Adds OpenSpec “propose” skill definition. |
| .github/skills/openspec-explore/SKILL.md | Adds OpenSpec “explore” skill definition. |
| .github/skills/openspec-archive-change/SKILL.md | Adds OpenSpec “archive change” skill definition. |
| .github/skills/openspec-apply-change/SKILL.md | Adds OpenSpec “apply change” skill definition. |
| .github/prompts/opsx-propose.prompt.md | Adds /opsx:propose prompt content. |
| .github/prompts/opsx-explore.prompt.md | Adds /opsx:explore prompt content. |
| .github/prompts/opsx-archive.prompt.md | Adds /opsx:archive prompt content. |
| .github/prompts/opsx-apply.prompt.md | Adds /opsx:apply prompt content. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| } | ||
|
|
||
| void xraudio_atomic_int_set(xraudio_atomic_int_t *atomic, int new_val) { | ||
| return atomic_store(atomic, new_val); |
There was a problem hiding this comment.
atomic_store returns void, so return atomic_store(...) is not valid C and may mislead readers copying the snippet. Update the sample to call atomic_store(atomic, new_val); without returning a value.
| return atomic_store(atomic, new_val); | |
| atomic_store(atomic, new_val); |
|
|
||
| ### Development Configuration | ||
| **JSON Configuration for Debug Features**: | ||
| ```json |
There was a problem hiding this comment.
This block is labeled as json but includes // comments and a trailing comma, making it invalid JSON. Either remove comments/trailing commas, or relabel the fence to jsonc (and keep it consistent across docs) so readers don’t copy/paste invalid configuration.
| ```json | |
| ```jsonc |
|
|
||
| ### Debug Configuration | ||
|
|
||
| ```json |
There was a problem hiding this comment.
The snippet is presented as strict JSON but contains a trailing comma at line 517, which makes it invalid JSON. Remove the trailing comma (or switch to jsonc if you want to allow JSON-with-comments style in the docs).
| ```json | |
| ```jsonc |
| # Enable required protocols | ||
| option(HTTP_ENABLED "Enable HTTP voice recognition" ON) | ||
| option(WS_ENABLED "Enable WebSocket voice recognition" ON) | ||
|
|
||
| # Configure build | ||
| add_definitions(-DHTTP_ENABLED -DWS_ENABLED) |
There was a problem hiding this comment.
This is likely to confuse integrators: option() is for configuring the project defining the option (i.e., the SDK build), while consumers typically enable features by configuring the SDK build (e.g., cmake -DHTTP_ENABLED=ON) or by using target_compile_definitions() on the SDK target. Consider updating this section to show the recommended way to enable SDK features (configure-time cache options for building the SDK) and avoid add_definitions() in consumer projects.
| # Enable required protocols | |
| option(HTTP_ENABLED "Enable HTTP voice recognition" ON) | |
| option(WS_ENABLED "Enable WebSocket voice recognition" ON) | |
| # Configure build | |
| add_definitions(-DHTTP_ENABLED -DWS_ENABLED) | |
| # When configuring the XR Voice SDK build, enable required protocols: | |
| # (run this from the XR Voice SDK build directory) | |
| # | |
| # cmake -DHTTP_ENABLED=ON -DWS_ENABLED=ON .. | |
| # | |
| # This configures the SDK itself to build with HTTP and WebSocket support. | |
| # In your application CMakeLists.txt, you can optionally propagate the same | |
| # feature macros to your target: | |
| target_compile_definitions(voice_app PRIVATE HTTP_ENABLED WS_ENABLED) |
| // PPR (Pre/Post Processing) Plugin Configuration | ||
| if(obj->ppr_plugin != NULL) { | ||
| jppr_config = json_object_get(json_obj_input, JSON_OBJ_NAME_INPUT_PPR); | ||
| if(NULL == jppr_config) { | ||
| XLOGD_INFO("PPR config not found, using defaults"); | ||
| } else if(!json_is_object(jppr_config)) { | ||
| XLOGD_INFO("jppr_config is not object, using defaults"); | ||
| jppr_config = NULL; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Create plugin objects with their specific configurations | ||
| if(obj->eos_plugin != NULL) { | ||
| obj->obj_eos = obj->eos_plugin->object_create(false, jeos_config); | ||
| } | ||
| if(obj->ppr_plugin != NULL) { | ||
| obj->obj_ppr = obj->ppr_plugin->object_create(jeos_config, jppr_config); | ||
| } |
There was a problem hiding this comment.
jppr_config is used without being declared in this snippet (only jeos_config is declared earlier). Since the document emphasizes validation against source, this undermines copy/paste correctness; declare json_t *jppr_config = NULL; in the example and ensure the object_create arguments reflect the real API (the first argument being jeos_config looks suspicious for a PPR create call).
| #### Output Operations | ||
| - **File Playback**: WAV, MP3, and other container format support | ||
| - **Memory Playback**: Direct buffer playback with format specification | ||
| - **Pipe/FIFO Playback**: Stream-based playbook from external sources |
There was a problem hiding this comment.
Corrected spelling of 'playbook' to 'playback'.
| - **Pipe/FIFO Playback**: Stream-based playbook from external sources | |
| - **Pipe/FIFO Playback**: Stream-based playback from external sources |
| #define XRSR_PROTOCOL_HTTP_BUFFER_SIZE_MAX (102400) // 100KB maximum buffer | ||
|
|
||
| typedef struct { | ||
| char write_buffer[XRSR_PROTOCOL_HTTP_BUFFER_SIZE_MAX]; |
There was a problem hiding this comment.
This document later defines xrsr_state_http_t again with different fields (debug flag), which creates ambiguity about the actual struct shape. Consider consolidating into a single snippet (or use // ... within one definition) so readers don’t encounter two conflicting typedefs with the same name.
| char write_buffer[XRSR_PROTOCOL_HTTP_BUFFER_SIZE_MAX]; | |
| char write_buffer[XRSR_PROTOCOL_HTTP_BUFFER_SIZE_MAX]; | |
| bool debug; |
No description provided.