Add IM880B as a new target platform #10681

itziardelatorre · 2019-05-27T16:27:08Z

Description

Adds IM880B as a new platform.

The iM880B is a compact and low-cost radio module that operates in the unlicensed 868 MHz band and combines a powerful Cortex-M3 controller with the new LoRa® transceiver SX1272 of Semtech Corporation.
More details available at https://wireless-solutions.de/products/radiomodules/im880b-l.html

Please find attached the corresponding test results.
2019-05-24-TESTS_IM880B.zip

Pull request type

[ ] Fix
[ ] Refactor
[X] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

Release Notes

ciarmcom · 2019-05-27T17:00:26Z

@itziardelatorre, thank you for your changes.
@ARMmbed/mbed-os-maintainers @ARMmbed/mbed-os-tools please review.

0xc0170 · 2019-05-28T09:44:42Z

Please review Travis, device_name not found?

itziardelatorre · 2019-05-28T10:03:03Z

Could you please give me more details about the information that is missing?
The new target information is added in target.json and the corresponding TARGET folder has been added too. Is any modification in the travis files also required?

itziardelatorre · 2019-05-28T11:52:49Z

device_name updated.

Could you please run the tests again?

0xc0170 · 2019-05-31T13:03:03Z

Seems travis passed, I'll run CI soon

0xc0170

Styling changes requested, otherwise it looks fine

0xc0170 · 2019-05-31T13:04:18Z

targets/TARGET_STM/TARGET_STM32L1/TARGET_IM880B/PinNames.h

+
+    //Standardized button name
+    SW1 		= PB_15,
+    BUTTON1 	= SW1,


small style change - some pins are not aligned - can you fix that ? It will cause "space style errors" in the future that we should avoid

Styling changes fixed.

0xc0170 · 2019-05-31T13:05:51Z

targets/TARGET_STM/TARGET_STM32L1/TARGET_IM880B/PeripheralPins.c

+/* mbed Microcontroller Library
+ *******************************************************************************
+ * Copyright (c) 2016, STMicroelectronics
+ * All rights reserved.


I understand these files were copied from elsewhere or other target. Can you add SPDX identifier for new files (we should still fix the old files). If you look at SPI.h or other files in platform folder, they contain

SPDX-License-Identifier: Apache-2.0 , in this case it would be BSD I believe. please add to the new files

Thanks for your feedback.

Is the "SPDX-License-Identifier: BSD-3-Clause" identifier correct?
Is this only for the "PeripheralPins.c" file required?

Any new file should contain it (it's stated in our license docs guide).

Yes, BSD-3-Clause should be the one for this file

SPDX-License-Identifier added

0xc0170 · 2019-06-03T11:14:23Z

Can you change the commit msg - not all of them are being about "adding new platform" but rather fixing the platform - first line could be "IM880B: fix style" or similar

itziardelatorre · 2019-06-03T11:15:56Z

I added in the second line the details. E.g.

"Add IM880B as a new target platform

SPDX-License-Identifier added"

Is this not enough?

0xc0170 · 2019-06-03T11:22:50Z

I would propose to have first commit as it is "Add IM880B as a new target platform" but the following commits to contain first lines to reflect what has changed. Simple git rebase -i should do the trick quickly to edit commit msgs for the last 3 commits.

If you need assistance , let us know

itziardelatorre · 2019-06-03T11:45:05Z

I have modified the last message. Could you please continue with the PR?

0xc0170 · 2019-06-03T12:05:41Z

What is the merge commit, can you remove that one please? This does not require any merge commits (should not contain them).

0xc0170 · 2019-06-03T12:15:56Z

I've rebased this here https://github.com/0xc0170/mbed-os/pull/new/new_target_IM880B (I can push to your branch if this looks good)

itziardelatorre · 2019-06-03T12:19:05Z

This looks fine. Thanks.

0xc0170 · 2019-06-03T12:30:16Z

Your branch was updated, will start CI once CI is fixed (there's currently internal issue)

itziardelatorre · 2019-06-03T12:30:44Z

Many thanks for your support

0xc0170 · 2019-06-04T13:03:53Z

CI started

mbed-ci · 2019-06-04T13:49:44Z

Test run: FAILED

Summary: 6 of 7 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_mbed2-build-ARM
jenkins-ci/mbed-os-ci_build-IAR
jenkins-ci/mbed-os-ci_mbed2-build-GCC_ARM
jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_mbed2-build-IAR

0xc0170 · 2019-06-04T14:10:37Z

CI was aborted, restarted again

mbed-ci · 2019-06-04T17:14:41Z

Test run: FAILED

Summary: 1 of 7 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM

itziardelatorre · 2019-06-05T06:44:48Z

The last test failed. Is any update from my side required?

0xc0170 · 2019-06-05T07:14:46Z

@itziardelatorre Please review build artifacts. ARM fails with errors:

        Link: /usr/local/armcc6.11/bin/armlink --via ./BUILD/tests/IM880B/ARM/./TESTS/network/l3ip/.link_options.txt
        [DEBUG] Return: 1
        [Warning] @0,0: L3912W: Option 'legacyalign' is deprecated.
        [Error] @0,0: L6218E: Undefined symbol Image$$ARM_LIB_STACK$$ZI$$Base (referred from BUILD/tests/IM880B/ARM/platform/mbed_retarget.o).
        [Error] @0,0: L6218E: Undefined symbol Image$$ARM_LIB_STACK$$ZI$$Length (referred from BUILD/tests/IM880B/ARM/rtos/TARGET_CORTEX/TOOLCHAIN_ARM_STD/mbed_boot_arm_std.o).
        [DEBUG] Errors: Warning: L3912W: Option 'legacyalign' is deprecated.
        [DEBUG] Errors: Error: L6218E: Undefined symbol Image$$ARM_LIB_STACK$$ZI$$Base (referred from BUILD/tests/IM880B/ARM/platform/mbed_retarget.o).
        [DEBUG] Errors: Error: L6218E: Undefined symbol Image$$ARM_LIB_STACK$$ZI$$Length (referred from BUILD/tests/IM880B/ARM/rtos/TARGET_CORTEX/TOOLCHAIN_ARM_STD/mbed_boot_arm_std.o).

Some symbols from boot sequence are missing in the linker script file?

itziardelatorre · 2019-06-05T07:37:03Z

Thanks for the information. I have updated the settings for the ARM compiler.

itziardelatorre · 2019-06-07T09:29:57Z

Is there any input from my side required to continue with the CI?
Thanks in advance.

0xc0170 · 2019-06-07T16:23:00Z

Is there any input from my side required to continue with the CI?

Please rebase, recent merge causing the conflicts now. we will retrigger CI (I actually was faster with CI rebuild than realizing there's merge conflict)

itziardelatorre · 2019-06-07T17:03:30Z

Please let me know if the update does not fix the conflict

itziardelatorre · 2019-06-11T12:30:09Z

Could you please confirm me if the conflict has been fixed?
Thanks.

itziardelatorre · 2019-06-18T09:15:52Z

Is there any input from my side required to continue with the CI?

Please rebase, recent merge causing the conflicts now. we will retrigger CI (I actually was faster with CI rebuild than realizing there's merge conflict)

Could you please confirm me if the conflict has been fixed?
Thanks.

itziardelatorre · 2019-07-04T08:35:28Z

@0xc0170 Could you please confirm me if the conflict has been fixed?
Is there any other issue open?

0xc0170 · 2019-07-04T10:19:06Z

@0xc0170 Could you please confirm me if the conflict has been fixed?

It still reports the conflict. Have you pushed to your branch? I don't see any new commit

itziardelatorre · 2019-07-04T11:06:26Z

@0xc0170 Thanks for your quick feedback.
Could you please check it again? I hope that the conflict is solved now.

0xc0170 · 2019-07-04T11:36:23Z

The latest commit looks incorrect to me (contains changes already on master) therefore there are conflicts.

itziardelatorre · 2019-07-04T11:43:43Z

How could I solve the conflicts? I have updated the last target.json from master with the new target information "IM880B".
What is the reason for the conflicts?

0xc0170 · 2019-07-04T12:09:49Z

I am rebasing this locally. The history looks wrong (line endings incorrect and the latest merges to resolve conflicts - rather use git rebase ).

itziardelatorre · 2019-07-04T12:17:33Z

Thanks for your support. Please let me know if you need any input from my side.

0xc0170 · 2019-07-04T12:38:05Z

I tried to clean it but still failing to get this conflict locally. I removed one commit that seemed to just change encoding of the files. I think the best would be to create new PR with clean history.

I can see here targets.json has most probably tab but locally I see spaces 🤕 I'll close this and @itziardelatorre please create new PR from a new branch, much easier.

0xc0170 · 2019-07-05T12:31:22Z

Resolved via #10972

Add IM880B as a new target platform

b4b9b52

ciarmcom requested a review from a team May 27, 2019 17:00

ciarmcom added the needs: review label May 27, 2019

0xc0170 added the scope: new-target label May 28, 2019

0xc0170 requested a review from MarceloSalazar May 28, 2019 09:44

0xc0170 requested changes May 31, 2019

View reviewed changes

itziardelatorre added 3 commits June 3, 2019 13:13

IM880B: update device_name

d5f5a4a

IM880B: Styling changes fixed

b571379

IM880B: SPDX-License-Identifier added

b3e4e4c

0xc0170 force-pushed the new_target_IM880B branch from 2f7473a to b3e4e4c Compare June 3, 2019 12:29

0xc0170 approved these changes Jun 3, 2019

View reviewed changes

0xc0170 added needs: CI and removed needs: review labels Jun 3, 2019

0xc0170 added needs: work and removed needs: CI labels Jun 5, 2019

0xc0170 added needs: CI and removed needs: work labels Jun 5, 2019

0xc0170 added needs: work and removed needs: CI labels Jun 7, 2019

0xc0170 force-pushed the new_target_IM880B branch from d54cf18 to b3e4e4c Compare July 4, 2019 11:57

0xc0170 force-pushed the new_target_IM880B branch from 9dc3a1f to b3e4e4c Compare July 4, 2019 12:13

0xc0170 force-pushed the new_target_IM880B branch from da9a335 to b3e4e4c Compare July 4, 2019 12:29

0xc0170 closed this Jul 5, 2019

itziardelatorre deleted the new_target_IM880B branch July 10, 2019 08:54

Add IM880B as a new target platform #10681

Add IM880B as a new target platform #10681

Uh oh!

Conversation

itziardelatorre commented May 27, 2019

Description

Pull request type

Reviewers

Release Notes

Uh oh!

ciarmcom commented May 27, 2019

Uh oh!

0xc0170 commented May 28, 2019

Uh oh!

itziardelatorre commented May 28, 2019

Uh oh!

itziardelatorre commented May 28, 2019

Uh oh!

0xc0170 commented May 31, 2019

Uh oh!

0xc0170 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Jun 3, 2019

Uh oh!

itziardelatorre commented Jun 3, 2019

Uh oh!

0xc0170 commented Jun 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

itziardelatorre commented Jun 3, 2019

Uh oh!

0xc0170 commented Jun 3, 2019

Uh oh!

0xc0170 commented Jun 3, 2019

Uh oh!

itziardelatorre commented Jun 3, 2019

Uh oh!

0xc0170 commented Jun 3, 2019

Uh oh!

itziardelatorre commented Jun 3, 2019

Uh oh!

0xc0170 commented Jun 4, 2019

Uh oh!

mbed-ci commented Jun 4, 2019

Uh oh!

0xc0170 commented Jun 4, 2019

Uh oh!

mbed-ci commented Jun 4, 2019

Uh oh!

itziardelatorre commented Jun 5, 2019

Uh oh!

0xc0170 commented Jun 5, 2019

Uh oh!

itziardelatorre commented Jun 5, 2019

Uh oh!

itziardelatorre commented Jun 7, 2019

Uh oh!

0xc0170 commented Jun 7, 2019

Uh oh!

itziardelatorre commented Jun 7, 2019

Uh oh!

itziardelatorre commented Jun 11, 2019

Uh oh!

itziardelatorre commented Jun 18, 2019

Uh oh!

0xc0170 commented Jun 3, 2019 •

edited

Loading