Skip to content

sys: simplify shell#3402

Merged
haukepetersen merged 4 commits intoRIOT-OS:masterfrom
kaspar030:simplify_shell
Sep 3, 2015
Merged

sys: simplify shell#3402
haukepetersen merged 4 commits intoRIOT-OS:masterfrom
kaspar030:simplify_shell

Conversation

@kaspar030
Copy link
Copy Markdown
Contributor

@kaspar030 kaspar030 commented Jul 14, 2015

seeing multiple different types of getchar/putchar given to shell, with no actual effect as all output goes to the default stdio anyways, I've decided to get rid of the parameters.

If we one day want shells with IO other than stdio, we should implement it right and redirect stdio.

This is breaks all platforms not offering getchar/putchar. (WIP because of this)

@kaspar030 kaspar030 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation State: waiting for other PR State: The PR requires another PR to be merged first labels Jul 14, 2015
@kaspar030 kaspar030 added State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet and removed State: waiting for other PR State: The PR requires another PR to be merged first labels Jul 14, 2015
haukepetersen
haukepetersen reviewed Jul 15, 2015
View reviewed changes
examples/rpl_udp/main.c Outdated
Copy link
Copy Markdown
Contributor

@haukepetersen haukepetersen Jul 15, 2015

Choose a reason for hiding this comment

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

where is line_buf defined?

Copy link
Copy Markdown
Contributor Author

@kaspar030 kaspar030 Jul 15, 2015

Choose a reason for hiding this comment

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

uhm, one line above? :)

@haukepetersen
Copy link
Copy Markdown
Contributor

haukepetersen commented Jul 15, 2015

very nice, +1!

@kaspar030
Copy link
Copy Markdown
Contributor Author

kaspar030 commented Jul 15, 2015

(I mark the PR "ready for CI" to see build errors)

@kaspar030 kaspar030 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jul 15, 2015
@haukepetersen
Copy link
Copy Markdown
Contributor

haukepetersen commented Jul 15, 2015

only some :-)

@kaspar030 kaspar030 mentioned this pull request Jul 16, 2015
Merged
@kaspar030
Copy link
Copy Markdown
Contributor Author

kaspar030 commented Jul 30, 2015

  • rebased
  • fixed chronos fix

@haukepetersen
Copy link
Copy Markdown
Contributor

haukepetersen commented Jul 31, 2015

could you pls squash?

@kaspar030
Copy link
Copy Markdown
Contributor Author

kaspar030 commented Jul 31, 2015

  • rebased
  • squashed

@kaspar030 kaspar030 removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Jul 31, 2015
@haukepetersen haukepetersen mentioned this pull request Aug 5, 2015
Closed
OlegHahm
OlegHahm reviewed Aug 5, 2015
View reviewed changes
sys/include/shell.h
Copy link
Copy Markdown
Member

@OlegHahm OlegHahm Aug 5, 2015

Choose a reason for hiding this comment

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

Shouldn't be 64 bytes enough?

Copy link
Copy Markdown
Contributor

@haukepetersen haukepetersen Aug 5, 2015

Choose a reason for hiding this comment

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

I would go with 128, its the safer bet...

Copy link
Copy Markdown
Member

@OlegHahm OlegHahm Aug 5, 2015

Choose a reason for hiding this comment

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

Ok, if I think of commands like fibroute you're probably right.

@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Aug 5, 2015

Shell doesn't work on MSB-A2.

@haukepetersen
Copy link
Copy Markdown
Contributor

haukepetersen commented Aug 5, 2015

#3530 should fix everything for the msba2

@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Aug 5, 2015

Ok, works on Z1 and pba-d-01-kw2x. Let's wait for #3530 then.

@haukepetersen
Copy link
Copy Markdown
Contributor

haukepetersen commented Aug 5, 2015

reviewing and testing it as we speak

@kaspar030
Copy link
Copy Markdown
Contributor Author

kaspar030 commented Aug 6, 2015

breaks msb430.

@haukepetersen
Copy link
Copy Markdown
Contributor

haukepetersen commented Aug 6, 2015

Will look into the msb430 anyway, so I try to deal with it.

@kaspar030
Copy link
Copy Markdown
Contributor Author

kaspar030 commented Aug 6, 2015

  • rebased

@OlegHahm OlegHahm added the State: waiting for other PR State: The PR requires another PR to be merged first label Aug 10, 2015
@kaspar030 kaspar030 mentioned this pull request Aug 11, 2015
Closed
@haukepetersen
Copy link
Copy Markdown
Contributor

haukepetersen commented Aug 13, 2015

MSP430 periph driver are under way, hope we can merge this this week...

@OlegHahm OlegHahm mentioned this pull request Aug 14, 2015
Closed
This was referenced Aug 21, 2015
Closed
Merged
@cgundogan
Copy link
Copy Markdown
Member

cgundogan commented Aug 25, 2015

needs a rebase, because of the ng_=> gnrc_ renaming

@kaspar030
Copy link
Copy Markdown
Contributor Author

kaspar030 commented Aug 26, 2015

  • rebased

@PeterKietzmann PeterKietzmann mentioned this pull request Aug 27, 2015
Merged
10 tasks
@kaspar030
Copy link
Copy Markdown
Contributor Author

kaspar030 commented Aug 28, 2015

-rebased

@kaspar030 kaspar030 added this to the Release 2015.08 milestone Sep 2, 2015
@kaspar030 kaspar030 removed the State: waiting for other PR State: The PR requires another PR to be merged first label Sep 3, 2015
@kaspar030
Copy link
Copy Markdown
Contributor Author

kaspar030 commented Sep 3, 2015

all dependencies are in!

  • rebased

@haukepetersen
Copy link
Copy Markdown
Contributor

haukepetersen commented Sep 3, 2015

code looks good, works on every board I tested (i.e msb-430h, nucleo-l1, arduino-due, avsextrem, msba2, yunjia-nrf) -> ACK

@haukepetersen
Copy link
Copy Markdown
Contributor

haukepetersen commented Sep 3, 2015

just for fun, also successfully tested for the frdm-k64f and the pba-d-01-kw2x

@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Sep 3, 2015

ACK

@haukepetersen
Copy link
Copy Markdown
Contributor

haukepetersen commented Sep 3, 2015

and go!

haukepetersen added a commit that referenced this pull request Sep 3, 2015
@haukepetersen
Merge pull request #3402 from kaspar030/simplify_shell
076e9db 
sys: simplify shell
@haukepetersen haukepetersen merged commit 076e9db into RIOT-OS:master Sep 3, 2015
@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Sep 3, 2015

Nice work, @kaspar030!

@kaspar030
Copy link
Copy Markdown
Contributor Author

kaspar030 commented Sep 3, 2015

Thanks, and thanks for reviewing!

@kaspar030 kaspar030 deleted the simplify_shell branch September 3, 2015 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@OlegHahm OlegHahm

Labels

CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Milestone

Release 2015.09

Development

Successfully merging this pull request may close these issues.

4 participants

@kaspar030 @haukepetersen @OlegHahm @cgundogan