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 i2cee-driver to components #10711

Merged
merged 22 commits into from Aug 30, 2019

Conversation

@geky
Copy link
Member

commented May 30, 2019

Description

From here #10495

I'm not going to be able to continue support what is the last external block device repo:
https://github.com/armmbed/i2cee-driver

So now it can either be brought into mbed-os like the rest of the block devices or someone else can support it out of tree. I've created this PR to help get it into mbed-os, since I think that would be the easiest option moving forward.

Pull request type

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

Reviewers

@SeppoTakalo, @pilotak, @rdrill, @ARMmbed/mbed-os-storage

Release Notes

Added i2cee-driver to components folder

geky and others added 16 commits Feb 14, 2017
Adding explicit type conversion to overloaded log function
mBed online compiler fix + struct example
option to pass I2C object
- Needed new gcc-arm-embedded ppa
- dist: Trusty did not allow apt-get update to work
- test.py is no longer working as it once was
- Needed to update pip versions
add missing get_type() from BlockDevice
…2806c11e0e05280c08db0b3ed6459b2d8dea2c'

git-subtree-dir: components/storage/blockdevice/COMPONENT_I2CEE
git-subtree-mainline: 9cc1caa
git-subtree-split: d92806c
@geky

This comment has been minimized.

Copy link
Member Author

commented May 30, 2019

Note: I don't know how much time I'm going to be able to spend maintaining this PR, so anyone is free to step in and update it.

@ciarmcom ciarmcom requested review from SeppoTakalo and ARMmbed/mbed-os-maintainers May 30, 2019
@ciarmcom

This comment has been minimized.

Copy link
Member

commented May 30, 2019

@geky, thank you for your changes.
@SeppoTakalo @ARMmbed/mbed-os-storage @ARMmbed/mbed-os-maintainers please review.

@ciarmcom ciarmcom requested a review from ARMmbed/mbed-os-storage May 30, 2019
@pilotak

This comment has been minimized.

Copy link
Contributor

commented May 30, 2019

Thanks @geky it works the same as before with the lib outside the os.

@0xc0170

This comment has been minimized.

Copy link
Member

commented Jun 4, 2019

@ARMmbed/mbed-os-storage Please review this component addition

@SeppoTakalo SeppoTakalo added needs: CI and removed needs: review labels Jun 13, 2019
@0xc0170 0xc0170 added the needs: CI label Aug 14, 2019
@0xc0170

This comment has been minimized.

Copy link
Member

commented Aug 14, 2019

CI restarted

@mbed-ci

This comment has been minimized.

Copy link

commented Aug 14, 2019

Test run: FAILED

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

Failed test jobs:

  • jenkins-ci/mbed-os-ci_pdmc-test
  • jenkins-ci/mbed-os-ci_greentea-test
@0xc0170

This comment has been minimized.

Copy link
Member

commented Aug 14, 2019

pdmc can be ignored (invalid) but test shows components-storage-blockdevice-component_i2cee-tests-block_device-i2cee failure, please review

@0xc0170 0xc0170 added needs: work and removed needs: CI labels Aug 14, 2019
@0xc0170

This comment has been minimized.

Copy link
Member

commented Aug 20, 2019

@geky how can we move this forward - any assistance needed ?

@0xc0170

This comment has been minimized.

Copy link
Member

commented Aug 26, 2019

We are getting close to 5.14 code freeze, what shall we do with this PR?

@pilotak

This comment has been minimized.

Copy link
Contributor

commented Aug 26, 2019

i have tested it physically on STM32F412ZG & STM32F303RE and it works perfectly it's just a matter of fixing the local test

@bulislaw

This comment has been minimized.

Copy link
Member

commented Aug 28, 2019

@geky ping Also how is that tested? Do we have any docs in the handbook?

@0xc0170

This comment has been minimized.

Copy link
Member

commented Aug 29, 2019

@ARMmbed/mbed-os-storage If no update made by later today, we'll remove 5.14 label and move this to the next minor version. We are close to the 5.14 code freeze.

@SeppoTakalo

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2019

Tests failed, because it still had I2CEE component in the K64F in the targets.json.
I removed it.
I don't know if any boards have this component on board, so it can automatically be tested in CI without adding extra components.

Tests provided seem OK for the component itself.

@0xc0170

This comment has been minimized.

Copy link
Member

commented Aug 29, 2019

CI started

@0xc0170 0xc0170 added needs: CI and removed needs: work labels Aug 29, 2019
@0xc0170

This comment has been minimized.

Copy link
Member

commented Aug 29, 2019

CI error, restarting

@mbed-ci

This comment has been minimized.

Copy link

commented Aug 29, 2019

Test run: FAILED

Summary: 2 of 4 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-ARM
  • jenkins-ci/mbed-os-ci_build-GCC_ARM
@0xc0170

This comment has been minimized.

Copy link
Member

commented Aug 29, 2019

CI was restarted

@mbed-ci

This comment has been minimized.

Copy link

commented Aug 29, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 3
Build artifacts

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Aug 29, 2019
@0xc0170

This comment has been minimized.

Copy link
Member

commented Aug 29, 2019

Before we integrate, remaining question:

  • tests
  • documentation

@SeppoTakalo @geky

@SeppoTakalo

This comment has been minimized.

Copy link
Contributor

commented Aug 30, 2019

Tests and documentation exist in the component folder itself.
No boards have this component in, so I'm relying only @pilotak 's report, that it works. Will not run actively in CI, but should probably be added in future.

Also, should be added to public documentation as well. I'll create ticket for my self, to do those in future.

@pilotak

This comment has been minimized.

Copy link
Contributor

commented Aug 30, 2019

I can confim is it working fine, i have just tried it on STM32F412ZG

@0xc0170

This comment has been minimized.

Copy link
Member

commented Aug 30, 2019

Thanks for the updates.

Tests and documentation exist in the component folder itself.
Also, should be added to public documentation as well. I'll create ticket for my self, to do those in future.

@bulislaw Shall this component be integrated and handbook gets updated via separate ticket?

@bulislaw

This comment has been minimized.

Copy link
Member

commented Aug 30, 2019

Yeah, lets do it.

@0xc0170 0xc0170 merged commit c74d67e into ARMmbed:master Aug 30, 2019
25 checks passed
25 checks passed
continuous-integration/jenkins/pr-head This commit looks good
Details
jenkins-ci/build-ARM Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage RTOS ROM(+0 bytes) RAM(+0 bytes)
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARM Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Success!
Details
travis-ci/docs Success!
Details
travis-ci/doxy-spellcheck Success!
Details
travis-ci/events Success! Runtime is 8593 cycles.
Details
travis-ci/gitattributestest Success!
Details
travis-ci/include_check Success!
Details
travis-ci/licence_check Success!
Details
travis-ci/littlefs Success! Code size is 8464B.
Details
travis-ci/psa-autogen Success!
Details
travis-ci/tools-py2.7 Success!
Details
travis-ci/tools-py3.5 Success!
Details
travis-ci/tools-py3.6 Success!
Details
travis-ci/tools-py3.7 Success!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.