Add necessary configs to support the EP_AGORA target #163
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@ARMmbed/team-embeddedplanet @maclobdell |
40Grit
reviewed
Sep 25, 2019
|
| So, I guess you could argue we don't need to And if we do need to remove those features here, id say lets make the smallest change possible to keep our UX clean. |
AnttiKauppila
suggested changes
Sep 26, 2019
|
@AnttiKauppila I'm not certain why, but the example failed if I didn't enable IPv6. Could be because we set the The Telit ME910 module we're using does support IPv6, so I would argue that it doesn't hurt to turn it on for our target. |
|
@trowbridgec You should of course enable IPv6 when needed, but your application config is not general for all users. There might be some other AGORA user who needs IPv4 with some different shield. We have chosen IPv4 as a default value for this example application and developers can override that value if needed. |
|
Also there is no more FEATURE_LWIP so no need to set "target.features_add". Or are you using some older version of Mbed OS which still has it? |
|
I would say this PR is a testament to the entire reason this example is now the default Pelion example. Pending ARMmbed/mbed-os#11566, if this PR is closed and the example builds for EP_AGORA then this has all been an excellent thought experiment. |
|
Oh this is the cellular example... either way, good thought experiment |
|
@AnttiKauppila Thanks for the hint regarding Regarding the With Without Any thoughts as to why that might be happening? |
|
@trowbridgec I think that is a real issue because the modem supports both IPv4 and 6 and DNS resolving fails for IPv4. This needs to be investigated |
|
@AnttiKauppila Can we merge this PR in until the issue was been investigated (then we can remove the setting |
|
I think IPv6 should always be enabled on cellular. Can you provide AT debug logs? |
|
With Without |
|
@trowbridgec Thanks for the logs. It's difficult to say actual reason, but as you have |
|
Agree. @chris, do it. |
|
@AriParkkila I'll make the change for all targets :) |
AriParkkila
approved these changes
Oct 3, 2019
|
@AnttiKauppila all good? @0xc0170 who runs maintenance and tests for examples? |
AnttiKauppila
approved these changes
Oct 4, 2019
|
Restarted tests |
|
Fixed target in Raas - smoke is passing now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds support for the Embedded Planet
EP_AGORAtarget to this example app.Relies on ARMmbed/mbed-os#11566