Skip to content

Return-based error-handling for shell handlers#2602

Merged
miri64 merged 1 commit intoRIOT-OS:masterfrom
kushalsingh007:shell-return
Mar 29, 2015
Merged

Return-based error-handling for shell handlers#2602
miri64 merged 1 commit intoRIOT-OS:masterfrom
kushalsingh007:shell-return

Conversation

@kushalsingh007
Copy link
Copy Markdown
Member

-Updated version of branch used by @authmillenon ( continuation of his work )
-Removed whitespace errors
-Part of fix for #708

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redundant check for number of arguments, already checked on L132

@OlegHahm OlegHahm added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. labels Mar 18, 2015
@miri64
Copy link
Copy Markdown
Member

miri64 commented Mar 18, 2015

Needs rebase. It also reverts #2582 #2581 and I found the following missing ports:

$ git grep "void [^ ]*(int argc, char \*\*argv)"
cpu/native/startup.c:__attribute__((constructor)) static void startup(int argc, char **argv)
examples/rpl_udp/rpl_udp.h:void rpl_udp_init(int argc, char **argv);
examples/rpl_udp/rpl_udp.h:void rpl_udp_set_id(int argc, char **argv);
examples/rpl_udp/rpl_udp.h:void rpl_udp_dodag(int argc, char **argv);
examples/rpl_udp/rpl_udp.h:void udp_server(int argc, char **argv);
examples/rpl_udp/rpl_udp.h:void udp_send(int argc, char **argv);
examples/rpl_udp/rpl_udp.h:void rpl_udp_ignore(int argc, char **argv);
pkg/openwsn/0005-fixes-to-RIOT-adaption.patch:+void openwsn_start_thread(int argc, char **argv) {
pkg/openwsn/0005-fixes-to-RIOT-adaption.patch:+void openwsn_start_thread(int argc, char **argv);
sys/shell/commands/sc_lps331ap.c:void _get_lps331ap_init_handler(int argc, char **argv)
sys/shell/commands/sc_lps331ap.c:void _get_lps331ap_read_handler(int argc, char **argv)
sys/shell/commands/sc_netif.c:void _netif_list(int argc, char **argv)
sys/shell/commands/sc_netif.c:void _netif_send(int argc, char **argv)
sys/shell/commands/sc_netif.c:void _netif_addr(int argc, char **argv)
sys/shell/commands/sc_netif.c:void _netif_chan(int argc, char **argv)
sys/shell/commands/sc_netif.c:void _netif_pan(int argc, char **argv)
sys/shell/commands/sc_transceiver.c:void _transceiver_get_set_pan_handler(int argc, char **argv)
sys/shell/commands/shell_commands.c:extern void _rtc_handler(int argc, char **argv);
sys/shell/commands/shell_commands.c:void _netif_list(int argc, char **argv);
sys/shell/commands/shell_commands.c:void _netif_send(int argc, char **argv);
sys/shell/commands/shell_commands.c:void _netif_addr(int argc, char **argv);
sys/shell/commands/shell_commands.c:void _netif_chan(int argc, char **argv);
sys/shell/commands/shell_commands.c:void _netif_pan(int argc, char **argv);

@miri64
Copy link
Copy Markdown
Member

miri64 commented Mar 19, 2015

(I made an error in the previous comment: I meant #2581)

@kushalsingh007
Copy link
Copy Markdown
Member Author

@authmillenon For your second last comment -

cpu/native/startup.c:__attribute__((constructor)) static void startup(int argc, char **argv)
pkg/openwsn/0005-fixes-to-RIOT-adaption.patch:+void openwsn_start_thread(int argc, char **argv) {
pkg/openwsn/0005-fixes-to-RIOT-adaption.patch:+void openwsn_start_th read(int argc, char **argv);

are the one's I was a little concerned about for the two patches I am not supposed to change them or do I have to resubmit the patch ?
for the cpu/native/startup.c code block , it used argc and **argv but then I on first though I think that they can be changed to int , as it uses usage_exit()

Can you explain what exactly is meant by reverting #2581 , does that mean I resolved the merge conflicts while rebasing incorrectly previously ?

@miri64
Copy link
Copy Markdown
Member

miri64 commented Mar 20, 2015

are the one's I was a little concerned about for the two patches I am not supposed to change them or do I have to resubmit the patch ?

Just try it and compile examples/default with USEPKG=openwsn, if the patch target does not fail, we are good. As far as I remember git is quite capable of handling changed patch-files.

for the cpu/native/startup.c code block , it used argc and **argv but then I on first though I think that they can be changed to int , as it uses usage_exit()

I think you must not concern yourself with this one. It's a normal UNIX process main (for the native process), so changing them might be harmful to the native port.

Can you explain what exactly is meant by reverting #2581 , does that mean I resolved the merge conflicts while rebasing incorrectly previously ?

Obviously, your changes apply to the version of the commands before this. #2581 reduced the shell handlers for MODULE_NG_NETIF to _netif_config and _netif_send, with _netif_config working similar as _net_if_ifconfig, just with ng_netif instead of net_if ;-)

@kushalsingh007 kushalsingh007 force-pushed the shell-return branch 2 times, most recently from a76170d to 900bc9e Compare March 21, 2015 13:45
@kushalsingh007
Copy link
Copy Markdown
Member Author

I tried compiling examples/default with USEPKG=openwsn however even without my changes to it , it failed to compile due to errors like ‘TIMER_1’ undeclared etc. And so I am skipping this in my recent commit.
Also I updated my branch so I think it won't revert the changes anymore.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Mar 21, 2015

@thomaseichinger do you know what this is about?

@kushalsingh007
Copy link
Copy Markdown
Member Author

All is well and @authmillenon the work that you suggested has been done(except for changing openwsn patch) , can you suggest what part to do next.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't get the point of this change, nor how it is related to return based error-handling.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Mar 24, 2015

Apart from #2602 (comment) and #2602 (comment) I'm fine with this PR

@kushalsingh007
Copy link
Copy Markdown
Member Author

@authmillenon : Made the changes you suggested in my latest commit.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Mar 24, 2015

For the future: please don't squash your changes immediately so it's easier for me to see your updates.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Mar 24, 2015

(moreover, travis is already kindof overheated, so I prefer if I can stop the build of out-dated commits ;-), which I can't if you overwrite them)

@miri64
Copy link
Copy Markdown
Member

miri64 commented Mar 24, 2015

Apart from that: ACK, when Travis is happy. Please squash (yes, you can overwrite my old commit with your's ;-))

- Included the missing parts.
- Squashed with @authmillenon's commit
@kushalsingh007
Copy link
Copy Markdown
Member Author

Done squashing . Thanks for letting me use your commit :) ( Now let's see how long Travis takes :P )

@miri64
Copy link
Copy Markdown
Member

miri64 commented Mar 25, 2015

Since this is a bigger change, I would prefer if we merge this before #2581 and #2582 #2583 ;).

@kushalsingh007
Copy link
Copy Markdown
Member Author

Kindly restart the build for this one :)

@miri64
Copy link
Copy Markdown
Member

miri64 commented Mar 29, 2015

I did already for a hundredth time ^^. There are either reachability issues with github or one of the apt-repositories. I'll keep the tab open :-)

@miri64
Copy link
Copy Markdown
Member

miri64 commented Mar 29, 2015

And go.

miri64 added a commit that referenced this pull request Mar 29, 2015
Return-based error-handling for shell handlers
@miri64 miri64 merged commit 3b42fae into RIOT-OS:master Mar 29, 2015
@miri64
Copy link
Copy Markdown
Member

miri64 commented Mar 29, 2015

@kushalsingh007 congratz for this one ;-)

@kushalsingh007
Copy link
Copy Markdown
Member Author

😃 Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants