gh #164 Update description of dsHdmiInGetStatus and dsHdmiInSelectPort#165
gh #164 Update description of dsHdmiInGetStatus and dsHdmiInSelectPort#165Dhivyailangovan wants to merge 12 commits intodevelopfrom
Conversation
include/dsHdmiIn.h
Outdated
| * Also, if thT port has an active connection, it should update isPresented to true as well. | ||
| * | ||
| * Also, if the port has an active connection, it should update isPresented to true as well. | ||
| * These Changes are applied asynchronously in a separate thread, and will be communicated through the |
There was a problem hiding this comment.
This statement shows that we are giving the implementation part rather than the specification. Can we make it like what we wanted from this function.
There was a problem hiding this comment.
Updated as per the comments.
New lines:
* Also, if the port has an active connection, it should update isPresented to true as well.
* Any change in the variable - 'dsHdmiInSignalStatus_t' will be communicated via the 'dsHdmiInStatusChangeCB_t' callback.
include/dsCompositeIn.h
Outdated
| * | ||
| * @note When a port is selected that port should be set as activePort in ::dsCompositeInStatus_t. | ||
| * Also, if there is a signal (ie isPortConnected[that port ID] is true), once active, isPresented should be set to true as well. | ||
| * These Changes are applied asynchronously in a separate thread, and will be communicated through the |
There was a problem hiding this comment.
Provide what we need from this API rather than how the implementation is considered.
There was a problem hiding this comment.
Updated as per the comments.
New lines:
* Any change in the variable - 'dsCompositeInStatus_t' will be communicated via the 'dsCompositeInStatusChangeCB_t' callback.
include/dsCompositeIn.h
Outdated
| * | ||
| * @pre dsCompositeInInit() should be called before calling this API. | ||
| * @pre dsCompositeInInit() should be called before calling this API. The Composite input port status is updated asynchronously via | ||
| * 'dsCompositeInStatusChangeCB_t' callback whenever a change is detected. |
There was a problem hiding this comment.
Can we provide what change is considered here? may be point to the structure dsCompInSignalStatus_t if we are considering these changes as part of this requirement.
There was a problem hiding this comment.
Updated as per the comments.
New lines:
* @pre dsCompositeInInit() should be called before calling this API.
* After every operation, that may change 'dsCompositeInStatus_t',should wait until the
* 'dsCompositeInStatusChangeCB_t' callback is received.
include/dsHdmiIn.h
Outdated
| * | ||
| * @pre dsHdmiInInit() must be called before calling this API. | ||
| * @pre dsHdmiInInit() must be called before calling this API. The HDMI input port status is updated asynchronously via | ||
| * 'dsHdmiInStatusChangeCB_t' callback whenever a change is detected. |
There was a problem hiding this comment.
The change is still a bit unclear, even on the callback description. Can you discuss with the team and suggest what changes are really considered here?
There was a problem hiding this comment.
Updated as per the comments.
New lines:
* After every operation, that may change 'dsHdmiInStatus_t',should wait until the
* 'dsHdmiInStatusChangeCB_t' callback is received.
a83fb0e
|
@Dhivyailangovan , always create PR against develop branch. I have now done the change for you, however you will need to resolve the conflicts now. |
|
Also, you have added Composite header files as well. Is this a new issue? If yes, can you raise a new ticket for composite and attach this PR to that as well or update it in the task ticket here(#164) Can you also rename the PR, so that it has github id and not jira id please |
There was a problem hiding this comment.
Pull request overview
Updates public header documentation to clarify that input status fields are updated asynchronously and observed via status-change callbacks, so callers should not assume immediate synchronous updates after selection/operations.
Changes:
- Add documentation guidance to wait for StatusChange callbacks after operations that may update status structs.
- Fix a typo in the HDMI-In
dsHdmiInSelectPortnote (“thT” → “the”). - Add similar asynchronous-status notes for Composite-In APIs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| include/dsHdmiIn.h | Documents async status updates and callback-based notification for HDMI-In status fields. |
| include/dsCompositeIn.h | Adds analogous async/callback documentation for Composite-In status behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * | ||
| * @note When a port is selected that port should be set as activePort in ::dsCompositeInStatus_t. | ||
| * Also, if there is a signal (ie isPortConnected[that port ID] is true), once active, isPresented should be set to true as well. | ||
| * Any change in the variable - 'dsCompositeInStatus_t' will be communicated via the 'dsCompositeInStatusChangeCB_t' callback. |
There was a problem hiding this comment.
PR title/description mention only HDMI-In APIs, but this change also updates Composite-In documentation in this file. Please either update the PR metadata to include Composite-In, or split this into a separate PR so the scope matches the description.
| * After every operation, that may change 'dsHdmiInStatus_t',should wait until the | ||
| * 'dsHdmiInStatusChangeCB_t' callback is received. |
There was a problem hiding this comment.
The new guidance about waiting for status updates is placed under @pre, which reads like a hard precondition for calling dsHdmiInGetStatus(). This is more accurately a @note/@attention about freshness of fields updated asynchronously. Also fix the punctuation/spacing (missing space after comma in "...,should").
| * After every operation, that may change 'dsHdmiInStatus_t',should wait until the | |
| * 'dsHdmiInStatusChangeCB_t' callback is received. | |
| * @note After every operation that may change ::dsHdmiInStatus_t, callers should wait until the | |
| * ::dsHdmiInStatusChangeCB_t callback is received before relying on the updated status. |
| * Also, if thT port has an active connection, it should update isPresented to true as well. | ||
| * | ||
| * Also, if the port has an active connection, it should update isPresented to true as well. | ||
| * Any change in the variable - 'dsHdmiInSignalStatus_t' will be communicated via the 'dsHdmiInStatusChangeCB_t' callback. |
There was a problem hiding this comment.
This note appears to reference the wrong type/callback: it says changes to 'dsHdmiInSignalStatus_t' are communicated via 'dsHdmiInStatusChangeCB_t'. In this header, signal status has its own callback type (dsHdmiInSignalChangeCB_t), while dsHdmiInStatusChangeCB_t carries dsHdmiInStatus_t. Please align the wording with the actual callback used for signal status vs overall status updates.
| * Any change in the variable - 'dsHdmiInSignalStatus_t' will be communicated via the 'dsHdmiInStatusChangeCB_t' callback. | |
| * Any change in the variable - 'dsHdmiInSignalStatus_t' will be communicated via the 'dsHdmiInSignalChangeCB_t' callback. |
| * @pre dsCompositeInInit() should be called before calling this API. | ||
| * After every operation, that may change 'dsCompositeInStatus_t',should wait until the | ||
| * 'dsCompositeInStatusChangeCB_t' callback is received. |
There was a problem hiding this comment.
Similar to dsHdmiInGetStatus(), the new "wait until ...StatusChangeCB" guidance is under @pre and reads like a strict precondition for calling dsCompositeInGetStatus(). Consider moving it to a @note/@attention about asynchronous updates, and fix punctuation/spacing (missing space after comma in "...,should").
| * @pre dsCompositeInInit() should be called before calling this API. | |
| * After every operation, that may change 'dsCompositeInStatus_t',should wait until the | |
| * 'dsCompositeInStatusChangeCB_t' callback is received. | |
| * @pre dsCompositeInInit() should be called before calling this API. | |
| * | |
| * @note After every operation that may change ::dsCompositeInStatus_t, applications should wait | |
| * until the ::dsCompositeInStatusChangeCB_t callback is received before relying on the | |
| * updated status. |
Need to update the description of dsHdmiInGetStatus and dsHdmiInSelectPort to explain below points
activePort and isPresented will be updated asynchronously in a separate thread, and will be communicated through the 'hdmiInStatusChangeCB' callback.