-
Notifications
You must be signed in to change notification settings - Fork 649
nxplayer: add more error messages #3117
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
base: master
Are you sure you want to change the base?
Conversation
d4b63d9
to
d4a514f
Compare
nxplayer currently only returns directly '-ENOENT', which makes it impossible to know the specific reason for the failure when testing audio with nxplayer. Therefore, I modified this part of the logic to print out the error as much as possible. Signed-off-by: Tang Meng <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @TangMeng12 :-)
- Please read the and update PR description accrodingly (see https://github.com/apache/nuttx/blob/master/CONTRIBUTING.md as instructed by the PR template).
- You don't have to close #3116 and start new PR when updates are necessary - just
git commit --amend
thengit push -f
and the same PR will get updated :-)
@@ -221,18 +220,35 @@ static int _open_with_http(const char *fullurl) | |||
|
|||
FAR struct hostent *he; | |||
he = gethostbyname(hostname); | |||
if (!he) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
he == NULL
?
|
||
memcpy(&server.sin_addr.s_addr, | ||
he->h_addr, sizeof(in_addr_t)); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these blank lines increase readability we can leave them untouched :-)
close(s); | ||
return -1; | ||
switch (err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please take a look what switch format is preferred https://nuttx.apache.org/docs/latest/contributing/coding_style.html#switch-statement
#else | ||
if ((pplayer->fd = open(pfilename, O_RDONLY)) == -1) | ||
pplayer->fd = open(pfilename, O_RDONLY); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When splitting here please also update other places (i.e. line 1859).
@@ -290,6 +290,22 @@ static int nxplayer_cmd_play(FAR struct nxplayer_s *pplayer, char *parg) | |||
printf("File %s not found\n", parg); | |||
break; | |||
|
|||
case ENETUNREACH: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#endif | ||
{ | ||
int err = errno; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why catch errno
return -1; | ||
switch (err) | ||
{ | ||
case 401: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you think errno contain 401/403?
return -1; | ||
switch (err) | ||
{ | ||
case ETIMEDOUT: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not return -err
nxplayer currently only returns directly '-ENOENT', which makes it impossible to know the specific reason for the failure when testing audio with nxplayer.
Therefore, I modified this part of the logic to print out the error as much as possible.
Note: Please adhere to Contributing Guidelines.
Summary
Update this section with information on why change is necessary,
what it exactly does and how, if new feature shows up, provide
references (dependencies, similar problems and solutions), etc.
Impact
Update this section, where applicable, on how change affects users,
build process, hardware, documentation, security, compatibility, etc.
Testing
Update this section with details on how did you verify the change,
what Host was used for build (OS, CPU, compiler, ..), what Target was
used for verification (arch, board:config, ..), etc. Providing build
and runtime logs from before and after change is highly appreciated.