Skip to content

Advance notice of folder structure change in under "/targets/TARGET_RENESAS" #4440

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

Closed
TomoYamanaka opened this issue Jun 5, 2017 · 17 comments

Comments

@TomoYamanaka
Copy link
Contributor

@mbedNoobNinja

Hello, we are Renesas mbed team.

We will release a new mbed board that mounts RZ/A1.
Related to that, we consider that would like to share common codes in the code below HAL in mbed-os.
The structure that we would like to change is under /targets/TARGET_RENESAS, we show its structure in the below.

New folder structure will be merged by end of Sep.
So, please let me know if there is any problem about new folder structure by end of Aug.
Also, we would appreciate it if you could execute the test by using new folder structure.

Current state:
The folder are separated by mbed board unit, and there are many same files in these folders.
https://github.com/ARMmbed/mbed-os/tree/master/targets/TARGET_RENESAS

proposed change:
The same files are shared, only difference files are separated in folder by mbed board unit.
In this way, maintenance and new board supporting will be very easy.
https://github.com/TomoYamanaka/mbed/tree/Feature_LYCHEE/targets/TARGET_RENESAS

@theotherjimmy
Copy link
Contributor

@TomoYamanaka That seems fine. I don't forseee the build system having any problems with that.

Would you also like an advance review of your changes to targets.json?

@theotherjimmy
Copy link
Contributor

master...TomoYamanaka:Feature_LYCHEE That's the diff with master. Your branch seems to be a bit behind, and it would not be accepted if you submitted a PR right now.

@TomoYamanaka
Copy link
Contributor Author

TomoYamanaka commented Jun 6, 2017

@theotherjimmy

Would you also like an advance review of your changes to targets.json?

Thank you.
Please let me know if there is any problem.
I will reflect its comments when I PR.

That's the diff with master. Your branch seems to be a bit behind, and it would not be accepted if you submitted a PR right now.

That's right. I understand that this branch is not up to date.
This branch is based on mbed-os-5.4.3 for some development.
For now, I will PR by end of Sep. At that time I will PR this branch after merge the latest master.

@mbedNoobNinja
Copy link
Contributor

@TomoYamanaka that's great, we are glad you are considering that.

Also, we would appreciate it if you could execute the test by using new folder

We will be able to run the tests not sooner than 2-3 weeks.

@TomoYamanaka
Copy link
Contributor Author

@mbedNoobNinja

Thank you.
We are looking forward to your reply.

@mbedNoobNinja
Copy link
Contributor

mbedNoobNinja commented Jun 27, 2017

Finally ...
Sorry for the delay here are the results for the master branch of @TomoYamanaka :
cli_tst_summary.txt
old_tst_summary.txt
here are the complete logs:
old_tst_arm_iar_gcc.txt
cli_tst_arm.txt
cli_tst_gcc.txt
cli_tst_iar.txt

Not bad, except the IAR ones ... but this is just for reference.

The branch in interest however is Feature_LYCHEE of @TomoYamanaka and here I am getting strange pyton error:
mbed test -m VK_RZ_A1H -t ARM
Traceback (most recent call last):
File "C:\Projects\kitVKRZA1H\mbed\tools\test.py", line 247, in
status = print_report_exporter.report(build_report)
File "C:\Projects\kitVKRZA1H\mbed\tools\test_exporters.py", line 92, in report
return self.exporter_print(test_summary_ext, print_log_for_failures=print_log_for_failures)
File "C:\Projects\kitVKRZA1H\mbed\tools\test_exporters.py", line 343, in exporter_print
raise Exception("'test_run' did not have a 'result' value")
Exception: 'test_run' did not have a 'result' value
[ERROR] 'test_run' did not have a 'result' value
[mbed] ERROR: "python" returned error code 1.
[mbed] ERROR: Command "python -u C:\Projects\kitVKRZA1H\mbed\tools\test.py -t ARM -m VK_RZ_A1H --source C:\Projects\kitVKRZA1H\mbed --build C:\Projects\kitVKRZA1H\mbed\BUILD\tests\VK_RZ_A1H\ARM --test-spec C:\Projects\kitVKRZA1H\mbed\BUILD\tests\VK_RZ_A1H\ARM\test_spec.json" in "C:\Projects\kitVKRZA1H\mbed"

The error is the same if I say:
mbed test -m RZ_A1H -t ARM

I am not really familiar with the mbed os testing procedure, the mbed 2 testing was more simple, but this looks far more advanced and complicated for me (maybe because I am too rookie with the python).
Anyway ... Any suggestions what to do next will be gladly appreciated !

@theotherjimmy
Copy link
Contributor

@TomoYamanaka @mbedNoobNinja The branch Feature_LYCHEE seems to be based on a release branch. We do not accept PRs to release branches at the moment, so I recommend that you base your work on master.

@mbedNoobNinja
Copy link
Contributor

... Well the error was in my TV! (more specifically in execution of switch command of the TortoiseGit) In attempt to switch from master to Feature_LYCHEE that mess up the whole project and cause the strange python issue. After fresh & clean checkout of the Feature_LYCHEE branch with TortoiseGit command (Git Clone) there were no python errors anymore.
... That's fine, but new issue come up:
[Error] i2c_api.c@64,0: #154: expression must have struct or union type
[Error] spi_api.c@47,0: #154: expression must have struct or union type
[Error] serial_api.c@151,0: #136: struct "serial_s" has no field "serial"
...Anyway after adding "I2C_ASYNCH", "SPI_ASYNCH" & "SERIAL_ASYNCH" interfaces as "device_has" ittems for the "VK_RZ_A1H" platform in targets.json, the errors disappeared and the test finally managed to run.

Here are the results:
xx_cli_tst_summary.txt
xx_old_tst_summary.txt
and the logs:
xx_cli_tst_arm.txt
xx_cli_tst_gcc.txt
xx_cli_tst_iar.txt
xx_old_tst_arm_iar_gcc.txt

Definitely there is some issue with the IAR binaries, it looks they stuck somewhere.
The other total failure is features-feature_lwip-tests-mbedmicro-net-tcp_hello_world

That's the situation for now ... any help will be welcome!

@TomoYamanaka
Copy link
Contributor Author

TomoYamanaka commented Jun 29, 2017

@mbedNoobNinja

Thank you for your comments, and I appreciate that you execute the test.

Summarizing the above, is it right that the rest problems is the below?

Definitely there is some issue with the IAR binaries, it looks they stuck somewhere.
The other total failure is features-feature_lwip-tests-mbedmicro-net-tcp_hello_world

This Feature_LYCHEE branch is based on the release branch of mbed-os-5.4.3.
So, can you execute the test again by using mbed-os-5.4.3?

@TomoYamanaka
Copy link
Contributor Author

@theotherjimmy

Thank you for your comments.

@TomoYamanaka @mbedNoobNinja The branch Feature_LYCHEE seems to be based on a release branch. We do not accept PRs to release branches at the moment, so I recommend that you base your work on master.

That's right.
This branch is based on release branch of mbed-os-5.4.3 for advance development.
Therefore, I will PR, after I merge Feature_LYCHEE branch to the latest master branch in advance.

@mbedNoobNinja
Copy link
Contributor

mbedNoobNinja commented Jun 29, 2017

I don't know how to explicitly checkout mbed-os-5.4.3.
The tags in the branch Feature_LYCHEE are:

feature_LYCHEE_rev1
feature_LYCHEE_rev2
feature_LYCHEE_rev3
mbed-os-5.1.0
mbed-os-5.1.0-rc1
mbed-os-5.1.0-rc2
mbed-os-5.1.0-rc3
mbed-os-5.1.0-rc4
mbed-os-5.1.0-rc5
mbed-os-5.2.0-rc1
mbed-os-5.3.0-rc1
mbed-os-5.4.0
mbed-os-5.4.0-rc1
mbed-os-5.4.0-rc2
mbed-os-5.4.1
mbed-os-5.4.2
mbed_lib_rev100
mbed_lib_rev101
mbed_lib_rev102
mbed_lib_rev103
mbed_lib_rev104
mbed_lib_rev105
mbed_lib_rev106
mbed_lib_rev107
mbed_lib_rev108
mbed_lib_rev110
mbed_lib_rev111
mbed_lib_rev112
mbed_lib_rev113
mbed_lib_rev114
mbed_lib_rev116
mbed_lib_rev117
mbed_lib_rev118
mbed_lib_rev119
mbed_lib_rev120
mbed_lib_rev121
mbed_lib_rev122
mbed_lib_rev138
mbed_lib_rev139
mbed_lib_rev65
mbed_lib_rev67
mbed_lib_rev68
mbed_lib_rev69
mbed_lib_rev70
mbed_lib_rev71
mbed_lib_rev73
mbed_lib_rev74
mbed_lib_rev75
mbed_lib_rev76
mbed_lib_rev77
mbed_lib_rev78
mbed_lib_rev79
mbed_lib_rev80
mbed_lib_rev81
mbed_lib_rev82
mbed_lib_rev83
mbed_lib_rev84
mbed_lib_rev85
mbed_lib_rev86
mbed_lib_rev87
mbed_lib_rev88
mbed_lib_rev89
mbed_lib_rev90
mbed_lib_rev91
mbed_lib_rev92
mbed_lib_rev93
mbed_lib_rev94
mbed_lib_rev95
mbed_lib_rev96
mbed_lib_rev97
mbed_lib_rev98
mbed_lib_rev99
mbed_liv_rev108
mbed_private_fsl_initial

There is no 5.4.3 version in it.
From official ARMmbed master's branch point of view, there is tag mbed-os-5.4.3 and if I checkout that the targets folder structure obviously won't have TARGET_RZA1XX. Should I run the test no matter what the folder structure is ?

If Yes, here are the results:
5.4.3_cli_tst_summary.txt
5.4.3_cli_tst_arm.txt
5.4.3_cli_tst_gcc.txt

features-feature_lwip-tests-mbedmicro-net-tcp_echo is fine, I accidentally pull the RJ cable out and the test failed !

I suppose if the old tests live in folder named unsupported they are not interesting anymore so I skipped them.

@theotherjimmy
Copy link
Contributor

I will PR, after I merge Feature_LYCHEE branch to the latest master branch in advance.

Please do not merge. There are several reasons not to:

  • We do not accept PRs to master with merge commits
  • You would be introducing duplicate history (master's and the release branch's)

Instead, you will have to cherry pick the commit range. You cannot rebase because there are duplicated commits on the release branch with different hashes, because they have a different ancestor. If you were to rebase, git would try to apply all of the release commits onto master in your branch. This is why I recomend doing development off master whenever possible: it allows you to rebase to bring things up to date.

@TomoYamanaka
Copy link
Contributor Author

@mbedNoobNinja

Thank you for test of mbed OS 5.4.3.
Sorry for lack of description.
Because I wanted to comfirm whether the new folder structure has affected, I asked for test to you.

As far as the test result, it seems that features-feature_lwip-tests-mbedmicro-net-tcp_hello_world test is fail in mbed OS 5.4.3 too.
I was relieved that the new folder structure did not seem to be affected.

@TomoYamanaka
Copy link
Contributor Author

@theotherjimmy

Thank you for your comments.

I am going to process as the follow.

  1. I rebase my master branch to latest.
  2. I merge only the code changing by Feature_LYCHEE branch to my master branch.
  3. I PR from my master branch.

Could you give me your opinion about the above?

@theotherjimmy
Copy link
Contributor

theotherjimmy commented Jun 30, 2017

@TomoYamanaka Sure. I'm happy to provide my opinion. There are two options depending on what you mean bay "latest". If you mean the tag "latest" then you will experience the same pitfalls I mentioned above, but with a more recent release branch. The "latest" tag points to the most recent release. If you meant "latest" to mean the most recent version of the master branch, then that sounds great (I have another recommendation, just a sec), but please avoid using the word "merge" when describing the process. In reality the git command that you would end up using for this process is either git rebase or git cherry-pick. I recommend, the following process, a variation of yours:

  1. Pull master from ARMmbed git checkout master; git pull armmbed master
  2. Rebase/Cherry-pick the commits from the Feature_LYCHEE branch git rebase --onto master mbed-os-5.4.3 Feature_LYCHEE
  3. PR from your Feature_LYCHEE

The advantage of this proccess, is that it leaves your master branch pristine. That would allow you to work on other PRs in the mean time.

@TomoYamanaka
Copy link
Contributor Author

TomoYamanaka commented Jul 3, 2017

@theotherjimmy

Thank you for your comments.
Sorry for lack of description.
The "latest" I mentioned is that the most recent version of the master branch when I PR.

And thank you for advicing too regard to describing the process.

@theotherjimmy
Copy link
Contributor

@TomoYamanaka You're welcome. Please let me know if you run into any issues when brining the branch up to the latest version of master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants