Donny9 commented on PR #18266:
URL: https://github.com/apache/nuttx/pull/18266#issuecomment-3839938947
> > test log is added, remove your requested change from pr.
>
> I asked for the logs because a large number of recent PRs from Xiaomi were
AI generated and this one contains some of the same indicators ("files changed"
section, poorly formatted text that look copy-pasted, random claims that have a
lot of repetition).
>
> I don't think the test shows a deadlock fix or a "~20-30% power reduction
in typical scenarios. You are right though that it does at least show nothing
visibly broken, so I won't block this PR further. But I am tired of these
sloppy (often AI-generated) PRs that make undemonstrated claims. I think the PR
description should have some kind of demonstrable proof of a performance
improvement or an issue fix if it's part of the argument for being merged.
The verification of this PR power consumption benefit requires a testing
environment where Android and Nuttx communicate and interact via RPMsg UORB.
Usually, this is an integrated testing environment, and this data cannot be
obtained through just unit testing. Therefore, we prefer to conduct unit
testing verification, such as the above UORB listener.
Please carefully review and focus on the modifications made to the patch
itself, rather than constantly making demands: provide test code. Because after
you thoroughly read the patch, you will find that the internal explanations
have already clarified why power consumption can be reduced, and the code
already has corresponding explanations:（avoid wakeup to savepower)
/* There are several scenarios that require broadcasting:
* 1. If the proxy corresponding to the EPT (Endpoint) did not
* exist previously, this constitutes the first broadcast.
* 2. If the proxy previously had a wakeup attribute, it should
* be broadcast every time.
* 3. If the proxy does not have the wakeup attribute and the
* target core is in the running state, we should still broadcast
* it.
*
* In summary, Let's avoid broadcasting non-wakeup sensors when the
* target core is in a sleep state to prevent unnecessary wakeups.
*/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
