Skip to content

Add support for EP_AGORA #21

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Add support for EP_AGORA #21

wants to merge 1 commit into from

Conversation

trowbridgec
Copy link

@trowbridgec trowbridgec commented Sep 10, 2019 •
edited
Loading

Adds support for the Embedded Planet Agora platform (EP_AGORA) to this demo application.

NOTE: This PR updates mbed-os.lib to master since there are necessary pinout changes to the EP_AGORA target which are set to be released in 5.14.0 in a couple days.

@anttiylitokola
Copy link
Contributor

Can one of the admins verify this patch?

@trowbridgec
Copy link
Author

@maclobdell @ARMmbed/team-embeddedplanet

@trowbridgec trowbridgec mentioned this pull request Sep 10, 2019
Closed
MarceloSalazar
MarceloSalazar reviewed Sep 11, 2019
View reviewed changes
MarceloSalazar
MarceloSalazar reviewed Sep 11, 2019
View reviewed changes
MarceloSalazar
MarceloSalazar reviewed Sep 11, 2019
View reviewed changes
MarceloSalazar
MarceloSalazar reviewed Sep 11, 2019
View reviewed changes
MarceloSalazar
MarceloSalazar reviewed Sep 11, 2019
View reviewed changes
MarceloSalazar
MarceloSalazar reviewed Sep 11, 2019
View reviewed changes
MarceloSalazar
MarceloSalazar suggested changes Sep 11, 2019
View reviewed changes
Copy link

@MarceloSalazar MarceloSalazar left a comment

Choose a reason for hiding this comment

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

@trowbridgec thanks for the PR. We'd like to keep a very simple application with no custom configuration.
Do you think would be possible to remove extra code and only introduce a json configuration a maybe a .lib file for the network driver?

@MarceloSalazar
Copy link

Note we don't (yet) have tests in place - depends on #13

FYI @maclobdell

@trowbridgec
Copy link
Author

@MarceloSalazar Thank you for taking time to review this PR!

Part of the reason that we didn't put the board initialization in mbed-os proper is that this target can be used with 3 different types of connectivity based on populate options (BLE, LoRa, cellular). So, we didn't want to hard-code support for cellular in the target definition for someone who is just using the BLE functionality, for example. You can see more info on our platform here.

We tried to keep the target definition as generic as possible and intended to "specialize" what's needed for the various mbed-os example applications (e.g. mbed-os-example-pelion, mbed-os-example-ble, mbed-os-example-cellular, mbed-os-example-lorawan, etc.).

I totally understand the desire to keep the test application to a minimum. I think I can move some of the special code to a COMPONENT following the example you gave, so I can start there. Do you have any other suggestions?

@MarceloSalazar MarceloSalazar added new platform WIP Work in progress labels Sep 25, 2019
@trowbridgec
Copy link
Author

Depends on ARMmbed/mbed-os#11566

@linlingao
Copy link

@trowbridgec The latest commit no longer builds. Looks like mbed os might have the wrong version???

Compile [ 17.9%]: arm_uc_pal_blockdevice.c
Compile [ 17.9%]: arm_uc_pal_blockdevice_implementation.c
[Error] mbed_config.h@298,98: 'MBED_ROM_SIZE' undeclared (first use in this function); did you mean 'MBED_ROM_START'?
[Warning] arm_uc_pal_blockdevice_implementation.c@493,28: overflow in implicit constant conversion [-Woverflow]
[ERROR] In file included from :0:0:
./mbed-cloud-client/update-client-hub/modules/pal-blockdevice/source/arm_uc_pal_blockdevice_implementation.c: In function 'pal_blockdevice_get_slot_addr_size':
././BUILD/EP_AGORA/GCC_ARM/mbed_config.h:298:98: error: 'MBED_ROM_SIZE' undeclared (first use in this function); did you mean 'MBED_ROM_START'?
#define MBED_CONF_UPDATE_CLIENT_STORAGE_SIZE ((MBED_ROM_START + MBED_ROM_SIZE - APPLICATION_ADDR) * MBED_CONF_UPDATE_CLIENT_STORAGE_LOCATIONS) // set by application[EP_AGORA]
^
./mbed-cloud-client/update-client-hub/modules/pal-blockdevice/source/arm_uc_pal_blockdevice_implementation.c:174:41: note: in expansion of macro 'MBED_CONF_UPDATE_CLIENT_STORAGE_SIZE'
MBED_CONF_UPDATE_CLIENT_STORAGE_SIZE);
^
././BUILD/EP_AGORA/GCC_ARM/mbed_config.h:298:98: note: each undeclared identifier is reported only once for each function it appears in
#define MBED_CONF_UPDATE_CLIENT_STORAGE_SIZE ((MBED_ROM_START + MBED_ROM_SIZE - APPLICATION_ADDR) * MBED_CONF_UPDATE_CLIENT_STORAGE_LOCATIONS) // set by application[EP_AGORA]
^
./mbed-cloud-client/update-client-hub/modules/pal-blockdevice/source/arm_uc_pal_blockdevice_implementation.c:174:41: note: in expansion of macro 'MBED_CONF_UPDATE_CLIENT_STORAGE_SIZE'
MBED_CONF_UPDATE_CLIENT_STORAGE_SIZE);
^
./mbed-cloud-client/update-client-hub/modules/pal-blockdevice/source/arm_uc_pal_blockdevice_implementation.c: In function 'ARM_UC_PAL_BlockDevice_Activate':
./mbed-cloud-client/update-client-hub/modules/pal-blockdevice/source/arm_uc_pal_blockdevice_implementation.c:493:28: warning: overflow in implicit constant conversion [-Woverflow]
result.error = FIRM_ERR_ACTIVATE;
^~~~~~~~~~~~~~~~~

[mbed] ERROR: "/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python" returned error.
Code: 1
Path: "/Users/lingao01/mbed-ep/mbed-os-example-pelion"
Command: "/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python -u /Users/lingao01/mbed-ep/mbed-os-example-pelion/mbed-os/tools/make.py -t gcc_Arm -m ep_agora --source . --build ./BUILD/EP_AGORA/GCC_ARM"
Tip: You could retry the last command with "-v" flag for verbose output

@trowbridgec
Copy link
Author

@linlingao Yes, that makes sense. This PR is now dependent on this PR in mbed-os: ARMmbed/mbed-os#11566.

@linlingao
Copy link

Builds fine now with #11566. Thanks!

teetak01
teetak01 suggested changes Sep 27, 2019
View reviewed changes
mbed_app.json Outdated
"target.components_add" : ["FLASHIAP"],
"update-client.bootloader-details" : "0x9884",
"update-client.application-details" : "0xB000",
"update-client.storage-address" : "(2*1024*1024)",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason this starts at 2 MB ?

Copy link
Author

Choose a reason for hiding this comment

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

This starts at 2MB because that example that I was following started at 2MB and now our Pelion applications use 2MB. I'm not sure that it needs to be at 2MB, but our NOR flash part is +4MB (depending on populate option) and the internal flash size of our application MCU (Nordic nRF52840) is 1MB, so it shouldn't be an issue.

@0Grit
Copy link

0Grit commented Sep 27, 2019 via email •
edited
Loading

@trowbridgec
Copy link
Author

@teetak01 I made some tweaks to this PR, can you please take another look at it? Thanks!

teetak01
teetak01 reviewed Oct 1, 2019
View reviewed changes
@teetak01
Copy link
Contributor

teetak01 commented Oct 1, 2019

@loverdeg-ep we will make this repo public, so you might consider editing your earlier post from email.

@trowbridgec
Copy link
Author

This PR relies on PR ARMmbed/mbed-os#11566 which was merged into master on mbed-os and is slated for release in mbed-os 5.14.1. Is it acceptable to update to mbed-os master now to push this PR through?

@teetak01
Copy link
Contributor

teetak01 commented Oct 1, 2019

We still need to get CI-automation support for this board, and get the official Mbed OS release updated here.

@MarceloSalazar

maclobdell
maclobdell approved these changes Oct 3, 2019
View reviewed changes
Copy link
Contributor

@maclobdell maclobdell left a comment

Choose a reason for hiding this comment

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

Approved pending the release of Mbed OS 5.14.1 which should have ARMmbed/mbed-os#11566 released in it.

@trowbridgec trowbridgec mentioned this pull request Oct 14, 2019
Closed
@teetak01
Copy link
Contributor

build-only

@trowbridgec
Copy link
Author

Bump, now that mbed-os 5.14.1 has been released (https://github.com/ARMmbed/mbed-os/releases/tag/mbed-os-5.14.1, which includes ARMmbed/mbed-os#11566), and CI boards have been received by @maclobdell

@teetak01
Copy link
Contributor

The boards are not yet part of CI.

teetak01
teetak01 reviewed Nov 21, 2019
View reviewed changes
@teetak01
Copy link
Contributor

@maclobdell @trowbridgec can you rebase this? I can then merge this in.

Could you also add this note to the README for this target:

"Build-only"

This effectively signifies that we only test the compilation for this target so far. We will remove the note after we get the board in CI.

@teetak01
Copy link
Contributor

run-ci

@teetak01
Copy link
Contributor

build-only

@iot-pdmc
Copy link
Collaborator

Test run: SUCCESS

Build artifacts

posted-by-tag: https://jenkins-client.isgtesting.com/job/mbed-os-example-pelion/

@wajahat-abbas wajahat-abbas mentioned this pull request Dec 2, 2019
Closed
@trowbridgec trowbridgec mentioned this pull request Dec 4, 2019
Merged
@trowbridgec trowbridgec closed this Dec 4, 2019
@trowbridgec
Copy link
Author

Closed due to issues when the repo was switched to public. New PR created at #68.

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

@teetak01 teetak01 teetak01 approved these changes

@0Grit 0Grit 0Grit left review comments

@maclobdell maclobdell maclobdell approved these changes

@MarceloSalazar MarceloSalazar Awaiting requested review from MarceloSalazar

Assignees

@MarceloSalazar MarceloSalazar

Labels
Missing hardware in CI new platform Requires CI wait-to-merge WIP Work in progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@trowbridgec @anttiylitokola @MarceloSalazar @linlingao @0Grit @teetak01 @iot-pdmc @maclobdell