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

Improve quick start documents for observation framework #62

Closed
yanj-github opened this issue Feb 8, 2024 · 17 comments
Closed

Improve quick start documents for observation framework #62

yanj-github opened this issue Feb 8, 2024 · 17 comments
Assignees
Labels
Blocker documentation Improvements or additions to documentation

Comments

@yanj-github
Copy link
Collaborator

yanj-github commented Feb 8, 2024

Currently, it is stated to run OF:
./analyse-recording.sh <mp4-filepath>

As far as I am aware we can run OF by passing additional arguments to it.
This will be useful for the user to run OF with additional arguments such as enable debug log set range of detection etc.
Please add to the document accordingly.

We will propose a suitable wording later.

@jpiesing jpiesing added the documentation Improvements or additions to documentation label Feb 13, 2024
@jpiesing
Copy link
Contributor

@FritzHeiden There seems to be a lot of confusion about running the OF "raw" on the host PC or in WSL or inside Docker and, when run in WSL, whether it's able to talk to the test runner that's running in Docker.

@FritzHeiden
Copy link
Collaborator

The motiviation behind runner the OF in the docker container is to reduce dependencies and to making the setup process easier. Is there any part in the current quick start that causes this confusion? Regarding the communication with the test runner, I will improve the quick start to be more specific on how this should work

@yanj-github
Copy link
Collaborator Author

@FritzHeiden from out end can you add the following please?

"Observation framework can be run with specific mode enabled by passing some optional arguments.
./analyse-recording.sh "
Please also add reference to "Additional Options" section from OF readme file.

@gitwjr
Copy link

gitwjr commented Feb 14, 2024

@FritzHeiden I suggest making it clear that the user needs to build the OF as the first step in Phase 3. It is not clear from the current wording this needs to be done (prior to build-dof.sh). "Following steps should be sufficient to get started with dockerized version, more details at https://github.com/cta-wave/device-observation-framework".

@jpiesing
Copy link
Contributor

@FritzHeiden I suggest making it clear that the user needs to build the OF as the first step in Phase 3. It is not clear from the current wording this needs to be done (prior to build-dof.sh). "Following steps should be sufficient to get started with dockerized version, more details at https://github.com/cta-wave/device-observation-framework".

The OF README describes how to install the OF outside docker so referring people to that is very confusing.

@FritzHeiden
Copy link
Collaborator

The updated documentation for phase 3 still wasn't merged. The updated version is now in master, please check again.

"Observation framework can be run with specific mode enabled by passing some optional arguments. ./analyse-recording.sh " Please also add reference to "Additional Options" section from OF readme file.

I can add this wording to the quick start. Would you consider giving the "Additional Options" section an anchor so I can link it directly? At the moment its just bold text.

@jpiesing
Copy link
Contributor

jpiesing commented Feb 14, 2024

I'm following the new QUICK_START and get an error as shown.

$ ./build-dof.sh
[+] Building 56.2s (10/13) docker:default
=> [internal] load .dockerignore 0.1s
=> => transferring context: 75B 0.0s
=> [internal] load build definition from Dockerfile.dof 0.1s
=> => transferring dockerfile: 810B 0.0s
=> [internal] load metadata for docker.io/library/python:3.8-bookworm 2.4s
=> [ 1/10] FROM docker.io/library/python:3.8-bookworm@sha256:9110b418dc2d04746f1ccc99c20fca1cb72e6f298e3bf0c20b6369f76e88967f 14.8s
=> => resolve docker.io/library/python:3.8-bookworm@sha256:9110b418dc2d04746f1ccc99c20fca1cb72e6f298e3bf0c20b6369f76e88967f 0.0s
=> => sha256:49b40be4436eff6fe463f6977159dc727df37cabe65ade75c75c1caa3cb0a234 64.14MB / 64.14MB 7.9s
=> => sha256:9110b418dc2d04746f1ccc99c20fca1cb72e6f298e3bf0c20b6369f76e88967f 1.86kB / 1.86kB 0.0s
=> => sha256:e88393a58f956e0c224a2489fa277944ef20603af906d5c1098d6bdcc1c7cf59 7.38kB / 7.38kB 0.0s
=> => sha256:7bb465c2914923b08ae03b7fc67b92a1ef9b09c4c1eb9d6711b22ee6bbb46a00 49.55MB / 49.55MB 1.9s
=> => sha256:2b9b41aaa3c52ab268b47da303015b94ced04a1eb02e58860e58b283404974f4 24.05MB / 24.05MB 1.4s
=> => sha256:625008535504ab68868ca06d1bdd868dee92a9878d5b55fc240af7ceb38b7183 2.01kB / 2.01kB 0.0s
=> => sha256:c558fac597f8ecbb7a66712e14912ce1d83b23a92ca8b6ff14eef209ab01aff2 211.12MB / 211.12MB 7.2s
=> => extracting sha256:7bb465c2914923b08ae03b7fc67b92a1ef9b09c4c1eb9d6711b22ee6bbb46a00 1.2s
=> => sha256:11402150a57e537c64dc69a28bba37f13acdedd50d8788894398a7b774786e7d 6.39MB / 6.39MB 2.2s
=> => sha256:4532b04ec8f74504e51d2e559a91696cf1d2abb690871b18983ccaf7f75800f9 17.28MB / 17.28MB 2.9s
=> => sha256:4df0efb76c10b517a5e00bb6e8d1215e7425e48c6897ba7e858da16de78daabf 245B / 245B 3.1s
=> => extracting sha256:2b9b41aaa3c52ab268b47da303015b94ced04a1eb02e58860e58b283404974f4 0.3s
=> => sha256:951fd003e72c32d376eaba82f18c2e31e52ade7a970c732bd189a59b68b6efee 2.85MB / 2.85MB 3.3s
=> => extracting sha256:49b40be4436eff6fe463f6977159dc727df37cabe65ade75c75c1caa3cb0a234 2.1s
=> => extracting sha256:c558fac597f8ecbb7a66712e14912ce1d83b23a92ca8b6ff14eef209ab01aff2 3.7s
=> => extracting sha256:11402150a57e537c64dc69a28bba37f13acdedd50d8788894398a7b774786e7d 0.2s
=> => extracting sha256:4532b04ec8f74504e51d2e559a91696cf1d2abb690871b18983ccaf7f75800f9 0.3s
=> => extracting sha256:4df0efb76c10b517a5e00bb6e8d1215e7425e48c6897ba7e858da16de78daabf 0.0s
=> => extracting sha256:951fd003e72c32d376eaba82f18c2e31e52ade7a970c732bd189a59b68b6efee 0.2s
=> [ 2/10] RUN apt-get update && apt-get install -y git libzbar0 netcat-openbsd libgl1 ffmpeg python3-pyaudio portaudio19-dev 17.2s
=> [ 3/10] WORKDIR /usr/app 0.0s
=> [ 4/10] RUN git clone https://github.com/cta-wave/device-observation-framework.git 1.3s
=> [ 5/10] WORKDIR /usr/app/device-observation-framework 0.0s
=> [ 6/10] RUN ./install.sh 19.9s
=> ERROR [ 7/10] RUN cd audio_mezzanine && ./import_audio_mezzanine.sh && cd .. 0.3s

[ 7/10] RUN cd audio_mezzanine && ./import_audio_mezzanine.sh && cd ..:
0.345 /bin/sh: 1: ./import_audio_mezzanine.sh: Permission denied


Dockerfile.dof:11

9 |
10 | RUN ./install.sh
11 | >>> RUN cd audio_mezzanine && ./import_audio_mezzanine.sh && cd ..
12 |
13 | RUN mkdir /usr/app/recordings

ERROR: failed to solve: process "/bin/sh -c cd audio_mezzanine && ./import_audio_mezzanine.sh && cd .." did not complete successfully: exit code: 126

This machine doesn't have the test runner installed on it but that won't be unusual as people will want to run the OF on faster machines or pass a failing test off to a more qualified engineer to analyse.

If this is because the test runner isn't installed and there's no way to avoid that then a more friendly error message should be added.

@yanj-github
Copy link
Collaborator Author

@jpiesing the error you have raised above is now resolved.

@yanj-github
Copy link
Collaborator Author

@FritzHeiden

I can add this wording to the quick start. Would you consider giving the "Additional Options" section an anchor so I can link it directly? At the moment its just bold text.

"Additional Options" section should be linkable directly now.

@yanj-github
Copy link
Collaborator Author

@gitwjr and @jpiesing I have added new issue #63 for connectivity issue from OF to test runner under WSL, as this is separate issue.

@jpiesing
Copy link
Contributor

@jpiesing the error you have raised above is now resolved.

Confirmed.

@jpiesing
Copy link
Contributor

I've reviewed this issue and all the comments. As far as I can see there's only one trivial change outstanding.

The link to the description of the options from the deploy README.md (renamed from QUICK_START.md) takes you to the OF repo. It doesn't take you to the text in the OF README.md that describes the options.

@FritzHeiden In the deploy README.md, please change the link "the documentation"

image

to be https://github.com/cta-wave/device-observation-framework/blob/main/README.md#additional-options

Then I think this issue can be closed.

@FritzHeiden
Copy link
Collaborator

done #72

@jpiesing
Copy link
Contributor

@yanj-github You created this issue. Please can you check if it can now be closed.

@jpiesing jpiesing assigned yanj-github and unassigned FritzHeiden Feb 20, 2024
@yanj-github
Copy link
Collaborator Author

yanj-github commented Feb 20, 2024

@jpiesing and @FritzHeiden I just noticed the deploy readme contains a section to record the capture.

position video recording device (e.g. smartphone with 120fps using AVC codec) in front of the display of DUT Note: Significant care is needed. Please see https://dash-large-files.akamaized.net/WAVE/assets/How-to-take-clear-recordings-v3.pptx .\

Can we link to "Obtain recording files" instead of just ppt file please? Obtain recording files contains more useful information for user must follow. And this is better in the future if we add another document for audio capture don't have to change the deploy readme again.

No further comments. Thanks

@FritzHeiden
Copy link
Collaborator

Done #74. Please check

@yanj-github
Copy link
Collaborator Author

Thank you very much @FritzHeiden Looks all good and closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocker documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

4 participants