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

Add a new target DELTA_DFCM_NNN40 #866

Merged
merged 14 commits into from
Feb 2, 2015
Merged

Add a new target DELTA_DFCM_NNN40 #866

merged 14 commits into from
Feb 2, 2015

Conversation

Marcomissyou
Copy link
Contributor

Hi,

There is a new target( DELTA_DFCM_NNN40 ) that we want to add into your platform.
We are Delta company. And We manufacture a BLE WiFi combo module.
We use NRF51822 BLE+M0 chip. It is configured to 32MHz X'tal and use 32K RC crystal oscillator.
The module supports OTA update FW so it is necessary to create another target to support OTA like Nordic nRF51822 FOTA.

Thanks
Marco

"disk":"F:\\",
"peripherals": ["24LC256"]
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This file should probably not be checked in

…erit from NRF51822, add SWIO default setting in system_nrf51822.c and SWIO,VERF pin in PinName.h
| (GPIO_PIN_CNF_INPUT_Disconnect << GPIO_PIN_CNF_INPUT_Pos)
| (GPIO_PIN_CNF_DIR_Output << GPIO_PIN_CNF_DIR_Pos);

NRF_GPIO->OUTCLR = (GPIO_OUTCLR_PIN19_Clear << GPIO_OUTCLR_PIN19_Pos);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the DELTA DFCM_NNN40 module, the pin 19 is connected a RF switch, that controls antenna to radiate Bluetooth 4.0 or WiFi. Pull low to select Bluetooth 4.0 and Pull high to select WiFi.
We define the default setting is pull low to select Bluetooth 4.0.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's mbed init which is called after the main, where you can implement specific target functionality, like this. I would move this gpo pin change there.

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 29, 2015

There are some missing pieces.

  • gpio pin target specific code move from SystemInit to mbed_sdk_init()
  • add export support for gcc arm (to be able to export a code and run tests for example)
  • online IDE runs on ARM compiler therefore ARM/uARM support is essential

You can send a separate pull request for above. I can merge this now as it is, but first, can you sign http://developer.mbed.org/contributor_agreement/ ?

@Marcomissyou
Copy link
Contributor Author

What do I need to do now? wait?
merge by myself?
Thank you very much!

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 30, 2015

I see you added those changes, can you please edit years in files you edited? Like in the mbed overrides file, it states 2014 but was created a day ago. Same applies to files you edited, extend the year.

One more, each toolchain has own script file where is a list of targets supported. Please add yours there, to gcc and uvision.

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 30, 2015

Please correct the indentation in target.py. The error from Travis build:

 self.supported_toolchains = ["ARM", "GCC_ARM"]
^
IndentationError: unexpected indent

Plus add your target here https://github.com/mbedmicro/mbed/blob/master/workspace_tools/export/gccarm.py and same for uvision

@Marcomissyou
Copy link
Contributor Author

Do I miss some thing?
Can I remove "self.supported_toolchains = ["ARM", "GCC_ARM"]"?
Like HRM1017 platform, we are the same, we all inherit NRF51822. and NRF51822 supported ARM and GCC_ARM.

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 30, 2015

Yes ,you can remove it

@Marcomissyou
Copy link
Contributor Author

Great! Thank you very much 0xc0170
What do I do next?

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 30, 2015

I'll merge this today.

You can test your exporters, if everything works from master, like blinky and some other tests which are there.

@Marcomissyou
Copy link
Contributor Author

ok, thanks

@Marcomissyou
Copy link
Contributor Author

I don't see our target on mbed repository yet.
has it merged?

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 2, 2015

It's going to be merged within minutes. I did not have time on Friday to finish merging, thanks for understanding.

0xc0170 added a commit that referenced this pull request Feb 2, 2015
Add a new target DELTA_DFCM_NNN40
@0xc0170 0xc0170 merged commit 9f80c90 into ARMmbed:master Feb 2, 2015
@ytsuboi
Copy link
Contributor

ytsuboi commented Feb 5, 2015

@Marcomissyou Hey, please be careful not to break other's platform. HRM1017 got problem by your pull request.

@Marcomissyou
Copy link
Contributor Author

Hi,

I apologize for that.
I think it happen in system_nrf51822.c.
#ifdef TARGET_DELTA_DFCM_NNN40 || TARGET_HRM1017Do I need to fix it?

Sorry

2015-02-06 2:17 GMT+08:00 ytsuboi notifications@github.com:

@Marcomissyou https://github.com/Marcomissyou Hey, please be careful
not to break other's platform. HRM1017 got problem by your pull request.


Reply to this email directly or view it on GitHub
#866 (comment).

@@ -67,7 +71,7 @@ void SystemInit(void)

// Start the external 32khz crystal oscillator.

#ifdef TARGET_HRM1017
#ifdef TARGET_DELTA_DFCM_NNN40 || TARGET_HRM1017
Copy link
Contributor

Choose a reason for hiding this comment

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

should be

#if defined(TARGET_DELTA_DFCM_NNN40) || defined(TARGET_HRM1017)

@ytsuboi
Copy link
Contributor

ytsuboi commented Feb 6, 2015

Yes, as @xiongyihui wrote, that is the point. The patch for this issue was already sent.
Please note, typical mbed library release is monthly. This means, HRM1017 users need to roll back mbed SDK version on online compiler for a month.
Please be careful to edit shared codes.

@ytsuboi
Copy link
Contributor

ytsuboi commented Feb 6, 2015

I wondered why preprocessor didn't notice, so tried reproduction.

[Warning] system_nrf51822.c@71: #14-D: extra text after expected end of preprocessing directive

It caused a warning! Why did you ignore this?
Do not place a pull-request until every error/warning was solved, please.

@Marcomissyou
Copy link
Contributor Author

Hi ytsuboi,

I am not familiar with github, this is my first time to use it and first pull request to mbed repository.
So I apologize again for this unfortunately situation.
I did not see the warning message. Please tell me where to check the warning message?
I will be more very carefully.
Sorry.

@ytsuboi
Copy link
Contributor

ytsuboi commented Feb 6, 2015

Okay. :-)
When you run workspace_tools\build.py, your compiler will show the message.

@Marcomissyou
Copy link
Contributor Author

Yes, I saw that warning.
Thank you for telling me.

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 6, 2015

I overlooked it also 😟

Addition to warnings checking, It is a good practise to keep checking pull requests, to minimize this type of issues as one pull request can impact more targets, or even common code. I keep PR open for a while, for comments.

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.

4 participants