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

chip-tool: subscribe command exits immediately, it needs timed run option #18800

Open
bluebin14 opened this issue May 25, 2022 · 15 comments
Open
Assignees

Comments

@bluebin14
Copy link
Contributor

bluebin14 commented May 25, 2022

Problem

chip-too subscribe command exits immediately, making it problematic to use in automated tests of all existing test plans that relate to subscription. Having an interactive mode does not solve the issue of automation.

Proposed Solution

Add option to run the tool for the duration specified. E.g. to run it for 30 seconds:
./chip-tool booleanstate subscribe state-value 1 10 30 12344321 1

@bluebin14 bluebin14 added the sve label May 25, 2022
@bzbarsky-apple
Copy link
Contributor

"timeout" is how long the caller of the command will wait before killing it off if it has not complete.

The "subscribe" command, in non-interactive mode, completes immediately. This has nothing to do with the timeout argument.

Is the actual request here for the "subscribe" command to not complete immediately? What would it do in automation exactly? When would it complete?

@bluebin14

@bluebin14
Copy link
Contributor Author

@bzbarsky-apple yes the request is for the subscribe command to not complete immediately but instead wait for a specified duration and exit. Without that the subscribe command is meaningless in non interactive mode. Since the timeout argument already exists and means "wait for at most x duration before exiting", indeed a different argument would be more appropriate that means "wait for at least x duration before exiting". Note that chip-tool in TE8 already had the option of "do not exit" although you could not specify a run duration.

@bluebin14 bluebin14 changed the title chip-tool: timeout argument is ignored, breaks test plan chip-tool: subscribe command exits immediately, it needs timed run option May 26, 2022
@bzbarsky-apple
Copy link
Contributor

I believe @vivien-apple purposefully removed the "do not exit" bit when interactive mode was implemented.... It really did not work well.

That said, adding a "wait N seconds after the subscription is established before exiting" option for subscriptions might not be too difficult....

@doublemis1
Copy link
Contributor

doublemis1 commented May 30, 2022

Also before interactive mode, the subscription keeps chip tool process until SIGTERM, using the following command:
./chip-tool basic subscribe node-label 1 100 1 0 --keepSubscriptions 1
or
./chip-tool basic subscribe-event leave 1 100 1 0 --keepSubscriptions 1

Can we restore this non-interactive chip tool feature?

@jnsgsr
Copy link

jnsgsr commented Jun 6, 2022

This is impediment for us to get results from preSVE testing in timely matter. Blocks automation of significant amount of test cases. Please increase the priority for a resolution of this.

@bzbarsky-apple
Copy link
Contributor

@jnsgsr Can you please describe how you would automate your test cases if subscribe did not quit immediately?

And can you not achieve the same effect by writing data to the stdin of chip-tool interactive start from your automation?

The reason this functionality was removed is that it was fundamentally pretty broken. If we're going to try to restore it, we need to understand how it would get used in practice, so we are sure it supports those uses, as well as whether those use cases are in fact unaddressed by interactive mode.

@jnsgsr
Copy link

jnsgsr commented Jun 7, 2022

@bzbarsky-apple We have several tests automated that are currently disabled because of this issue (some with high criticality) and it is blocking automation for many test cases where we have to test if an accessory sends events.
We were using it as intended, spawning a chip-tool process with subscribe or subscribe-event commands to work in the background and gather events, and we parse chip-tool output with specific patterns to get event parameters and to verify those in following test steps.
Interactive mode does not suit here, there is no user interaction required beside calling the initial command. For any other command I would just open another shell with chip-tool to have clear output from both commands.
When using interactive mode and after calling subscribe-event, between lines of output there is a prompt i.e. >>> and a bunch of redundant bytes around it, and there is no strict pattern as there are some lines that do not have those. This makes it more problematic to automate parsing.
If you decide not to fix it, let us know right away, as in our current plan we assumed we can wait for this to be fixed, and just re-enable the tests.

@bluebin14
Copy link
Contributor Author

bluebin14 commented Jun 7, 2022

stdin input handling could indeed use some tweaks, commands randomly either do not show up (all of them should be clearly echoed) or show up on every line even though issued exactly once.

>>> quit()    [1654586300470] [54089:614038] CHIP: [CTL] Shutting down the controller
>>> quit()    [1654586300470] [54089:614038] CHIP: [CTL] Shutting down the commissioner
>>> quit()    [1654586300470] [54089:614038] CHIP: [CTL] Shutting down the controller
>>> quit()    [1654586300470] [54089:614038] CHIP: [IN] Expiring all connections for fabric 1!!

@vivien-apple
Copy link
Contributor

vivien-apple commented Jun 8, 2022

So if my understanding is correct, you are interested in gatherings events received by chip-tool and you have some log parsing scripts that are checking against your expected results ?

It is possible to do something like ?

$ mkfifo /tmp/chip-tool-stdin
$ nohup ./out/debug/standalone/chip-tool interactive start --trace_file /tmp/subscribe.interactive.trace > /tmp/interactive.log.txt < /tmp/chip-tool-stdin &
$ BACKGROUND_TASK_PID=$!
$ echo "onoff subscribe on-off 1 5 0x12344321 1" > /tmp/chip-tool-stdin
# Wait for the specific duration you are interested in
$ kill $BACKGROUND_TASK_PID
# Now there is a log of the messages that have been emitted/received by chip-tool in /tmp/subscribe.onoff.trace. The payload are base64 TLV bytes. To get an easier to understand version it can be converted with another command
$ chip-trace-decoder --source /tmp/subscribe.onoff.trace > /tmp/subscribe.onoff.decoded.trace
# run your script on the decoded version

There is a little issue with the trace-decoder-program and status report at the moment (those are not decoded, I'm going to fix that)... (update: I have opened #19307 for it)

@bluebin14
Copy link
Contributor Author

This approach can get very awkward when e.g. doing subscriptions from multiple controllers (let's say 5) then stopping controller 3 after 10 seconds etc. With timed run you have 5 lines and one script. With interactive mode... still counting.There is nothing wrong in having an interactive mode but maybe its usage should not be forced.

@woody-apple
Copy link
Contributor

SDK Review: Removing sve label, @cjandhyala to follow up to seed if this is still an issue.

@woody-apple
Copy link
Contributor

SDK Review: @bluebin14 can you confirm to see if, or if not there are any known issues blocking SVE, or testing at this point?

@bluebin14
Copy link
Contributor Author

@woody-apple there are no known issues blocking SVE, the non-interactive mode timed run option for subscription test cases is a feature request for better automation capability outside the test harness. The test harness can have its own solution for those cases using interactive mode (not tested though).

@stale
Copy link

stale bot commented Jan 2, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale Stale issue or PR label Jan 2, 2023
@bzbarsky-apple bzbarsky-apple removed the stale Stale issue or PR label Jan 3, 2023
@stale
Copy link

stale bot commented Jul 11, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale Stale issue or PR label Jul 11, 2023
@bzbarsky-apple bzbarsky-apple removed the stale Stale issue or PR label Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants