Skip to content

Uhuru RAVEN: Adding platform HAL #9787

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 0 commits into from
Closed

Uhuru RAVEN: Adding platform HAL #9787

wants to merge 0 commits into from

Conversation

junichikatsu
Copy link
Contributor

Description

Adding new platform Uhuru RAVEN, STM32F767VI with ESP32 wi-fi module.

Test results:
GCC
ARM
IAR

Pull request type

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

jeromecoutant
jeromecoutant approved these changes Feb 21, 2019
View reviewed changes
targets/targets.json Outdated
"network-default-interface-type": "WIFI"
},
"detect_code": ["9020"],
"device_has_add": ["ANALOGOUT", "CAN", "LOWPOWERTIMER", "TRNG", "FLASH", "MPU"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

"LOWPOWERTIMER" can be removed (deprecated value)

targets/targets.json Outdated
"value": 1
}
},
"overrides": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I advice to add
"lpticker_delay_ticks": 4,
like for NUCLEO_F767ZI

targets/targets.json Outdated
@@ -3029,6 +3029,31 @@
"device_name": "STM32H743ZI",
"bootloader_supported": true
},
"UHURU_RAVEN": {
"inherits": ["FAMILY_STM32"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

I advice to add
"device_has_remove": ["SERIAL_FC"],
as you don't set any pins in PinMap_UART_RTS and PinMap_UART_CTS

@ciarmcom ciarmcom requested review from a team February 21, 2019 10:00
@ciarmcom
Copy link
Member

@junichikatsu, thank you for your changes.
@ARMmbed/mbed-os-core @ARMmbed/mbed-os-wan @ARMmbed/mbed-os-pan @ARMmbed/mbed-os-storage @ARMmbed/mbed-os-hal @ARMmbed/mbed-os-ipcore @ARMmbed/mbed-os-mesh @ARMmbed/mbed-os-maintainers @ARMmbed/mbed-os-test @ARMmbed/mbed-os-crypto @ARMmbed/mbed-os-tls please review.

dannybenor
dannybenor reviewed Feb 21, 2019
View reviewed changes
targets/targets.json Outdated
@@ -3029,6 +3029,31 @@
"device_name": "STM32H743ZI",
"bootloader_supported": true
},
"UHURU_RAVEN": {
Copy link

Choose a reason for hiding this comment

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

Any storage components that should be added? Any SD card, external or internal flash?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The RAVEN board has external SPI Flash.

@0xc0170 0xc0170 removed request for a team February 21, 2019 11:59
@0xc0170
Copy link
Contributor

0xc0170 commented Feb 21, 2019

Hi @junichikatsu , thanks for the pull request. We will review shortly.

I've noticed few merges, can they be squashed and make history nice linear with only few commits - the changes are simple here adding few files.

0xc0170
0xc0170 reviewed Feb 21, 2019
View reviewed changes
targets/TARGET_STM/TARGET_STM32F7/TARGET_STM32F767xI/TARGET_UHURU_RAVEN/uhuru_raven_init.h Outdated
@@ -0,0 +1,37 @@
/* mbed Microcontroller Library
* Copyright (c) 2019 Uhuru Corporation
Copy link
Contributor

Choose a reason for hiding this comment

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

For new files, can you add SPDX idenfier please?

SPDX-License-Identifier: Apache-2.0 in this file, for some ST related, they are BSD-3-Clause

@0xc0170 0xc0170 changed the title [Uhuru RAVEN] Adding platform HAL Uhuru RAVEN: Adding platform HAL Feb 21, 2019
@0xc0170
Copy link
Contributor

0xc0170 commented Feb 21, 2019

Adding new platform Uhuru RAVEN, STM32F767VI with ESP32 wi-fi module.

How is ESP32 tested, is it enabled?

@cmonr
Copy link
Contributor

cmonr commented Feb 21, 2019

'from ARMmbed/master'

@junichikatsu When opening PRs, be sure that no merged from the master branch exist. Because of the way our release process works, merges from master will often result in weird behavior, such as #9787 (comment)

If you haven't already, please review: https://os.mbed.com/docs/mbed-os/v5.11/contributing/workflow.html

Avoid merging commmits. (Always rebase when possible.)

@junichikatsu
Copy link
Contributor Author

Adding new platform Uhuru RAVEN, STM32F767VI with ESP32 wi-fi module.

How is ESP32 tested, is it enabled?

We plan to use the following library used in Renesas GR-LYCHEE
https://github.com/d-kato/esp32-driver

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 27, 2019

Note, #9571 - might have implications for new targets. Please review (there's design document, and targets updated, if any questions, let us know). In any case, please reply to this comment if this change is already done or when completed.

@deepikabhavnani
Copy link

This target is using existing linker files and not adding new, should be fine.

@cmonr
Copy link
Contributor

cmonr commented Mar 1, 2019

I think we're only waiting on this before we can start testing: #9787 (comment)

Also, still waiting on the PR's commit history to be merge-free: d97c867

Commit history should be free of merge commits from master, and branches should be updated via rebasing.

cmonr
cmonr suggested changes Mar 1, 2019
View reviewed changes
Copy link
Contributor

@cmonr cmonr left a comment

Choose a reason for hiding this comment

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

#9787 (comment)

donatieng
donatieng suggested changes Mar 4, 2019
View reviewed changes
targets/TARGET_STM/TARGET_STM32F7/TARGET_STM32F767xI/TARGET_UHURU_RAVEN/uhuru_raven_init.c Outdated
* MBED API FUNCTIONS
* ----------------------------------------------------------------*/

void uhuru_raven_init(void)
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the rationale behind hard coding initialization of pins here?

@junichikatsu junichikatsu mentioned this pull request Mar 13, 2019
Closed
@junichikatsu
Copy link
Contributor Author

I made a branch and issued the following pull request.
I'm sorry to trouble you, but please check here.

#10091

@cmonr
Copy link
Contributor

cmonr commented Mar 14, 2019

@junichikatsu I'm not sure I follow. Does #10091 superceed/replace this PR?

@cmonr
Copy link
Contributor

cmonr commented Mar 14, 2019

Also, this problem still hasn't been fixed: #9787 (comment)

@junichikatsu junichikatsu force-pushed the master branch 4 times, most recently from c3f3bcd to f8c2cb2 Compare March 14, 2019 04:04
@junichikatsu junichikatsu mentioned this pull request Mar 15, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xc0170 0xc0170 0xc0170 left review comments

@dannybenor dannybenor dannybenor left review comments

@cmonr cmonr cmonr requested changes

@donatieng donatieng donatieng requested changes

@yanesca yanesca yanesca approved these changes

@jeromecoutant jeromecoutant jeromecoutant approved these changes

@deepikabhavnani deepikabhavnani deepikabhavnani approved these changes

Assignees
No one assigned
Labels
scope: new-target
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@junichikatsu @ciarmcom @0xc0170 @cmonr @deepikabhavnani @donatieng @yanesca @dannybenor @jeromecoutant @adbridge