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 Host make run behaviour #1736

Merged
merged 8 commits into from
Jul 2, 2019

Conversation

mikee47
Copy link
Contributor

@mikee47 mikee47 commented Jul 1, 2019

With 0126e4e a better default forSMING_TARGET_OPTIONS is just --flashfile=$(FLASH_BIN) --flashsize=$(SPI_SIZE).

Removing --uart=0 --uart=1 defaults to console output for serial, but no input possible.

Removing --pause=5 runs the code without delay. In practice --pause (wait for keypress) is more useful. The 5s delay was a compromise so that integration tests could run unattended, no longer required.

Where serial I/O is required for a project, manually running telnet, etc. on each run is annoying. This PR now automatically launches the appropriate telnet sessions in new terminal windows and passes the correct options to the emulator. The project defines which ports are required by setting ENABLE_HOST_UARTID in their component.mk file.

Tested in Windows and Ubuntu.

SMING_TARGET_OPTIONS is no longer published, instead use other variables which are managed by their relevant Components (flash, uart, wifi, etc.)

Given the complexity make flash no longer runs the emulator, however this can be done using make flash run.

@mikee47 mikee47 changed the title Launch telnet sessions automatically as required for run target Launch telnet sessions automatically for Host run Jul 1, 2019
@slaff slaff added this to the 3.9.0 milestone Jul 1, 2019
@mikee47
Copy link
Contributor Author

mikee47 commented Jul 1, 2019

I'm wondering if we need the telnet target at all now?

@slaff
Copy link
Contributor

slaff commented Jul 1, 2019

I'm wondering if we need the telnet target at all now?

I guess in the cases where a developer needs both uart 0 and uart 1. As for example in the Basic_Serial sample.

@slaff
Copy link
Contributor

slaff commented Jul 1, 2019

I have tried this PR and as far as I can see now there is a difference between make run and make flash related to serial output. Is that intentional?

If I run make run it starts the application and opens two new terminal windows. But if I run make flash it automatically starts sending uart0 to the same terminal and there is no uart1 output(or server listening for telnet connection).

@mikee47
Copy link
Contributor Author

mikee47 commented Jul 1, 2019

That is a good point. I figured that make flash should also run the emulator, but if we follow that logic then flashapp should do the same. However, I have no idea what a flashapp target might actually do for Host, so I left it out and instead added run.

I think it might be less confusing if make flash ONLY updates the virtual flash image. If we want to run it as well just do make flash run. Or make flash gdb - which is actually a bit trickier to deal with and at present terminals, etc. need to be handled manually.

Also, the telnet target isn't very helpful. It's just as easy (and more flexible) to type telnet 10000, etc.

@mikee47
Copy link
Contributor Author

mikee47 commented Jul 1, 2019

OK, I've updated make gdb so it starts up uart servers as for make run but then pauses so user can startup telnet, etc. if required.

@mikee47
Copy link
Contributor Author

mikee47 commented Jul 1, 2019

Haven't considered all this in terms of debugging via eclipse. Could maybe create a command file dynamically... Any thoughts?

@mikee47 mikee47 changed the title Launch telnet sessions automatically for Host run Improve Host make run behaviour Jul 1, 2019
# Options to add for configuring host network behaviour
CACHE_VARS += HOST_NETWORK_OPTIONS
HOST_NETWORK_OPTIONS ?=
SMING_TARGET_OPTIONS += $(HOST_NETWORK_OPTIONS)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you can rename SMING_TARGET_OPTIONS to CLI_TARGET_OPTIONS. Let's use the prefix SMING_ for important things like SMING_HOME and SMING_ARCH.

@slaff
Copy link
Contributor

slaff commented Jul 2, 2019

@mikee47 Is this PR ready for merging?

@mikee47
Copy link
Contributor Author

mikee47 commented Jul 2, 2019

Yes, all done.

@slaff slaff merged commit c6800a9 into SmingHub:develop Jul 2, 2019
@slaff slaff removed the 3 - Review label Jul 2, 2019
@mikee47 mikee47 deleted the update/Host_auto_uart branch July 2, 2019 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants