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

Fix lazy init and prevent un-necessary calls to NativeInit #981

Merged
merged 15 commits into from
Nov 14, 2018

Conversation

sharmavishnu
Copy link
Contributor

@sharmavishnu sharmavishnu commented Nov 11, 2018

Stub changes due to change in I2C core library

Description

Everytime "I2cDevice.FromId" method is called, the "NativeInit" method gets called. This method should be called only once per I2C bus. This fix ensures that NativeInit is called only when a new I2C bus is used in managed code

Motivation and Context

  1. Improve code
  2. Another attempt to fix I2C hang issue as reported in ToString() causing hardfault Home#416 and Debugging SPI with single-stepping fails for second time onwards Home#424

How Has This Been Tested?

Tested with HTU21D I2C sensor. So far have been able to debug multiple times with no I2C hang.

Screenshots

Types of changes

  • Improvement (non-breaking change that improves a feature, code or algorithm)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Signed-off-by: @sharmavishnu

sharmavishnu and others added 7 commits October 20, 2018 21:34
Part of changes made to the I2C managed library resulting in stub changes
Changes resulting due to I2C library changes. I'm not too sure how version and checksum are getting updated. Updated just the checksum as generated on my build machine. Please double-check and educate me on this aspect
@nfbot
Copy link
Member

nfbot commented Nov 11, 2018

Hi @sharmavishnu,

I'm nanoFramework bot.
Thank you for your contribution!

A human will be reviewing it shortly. 😉

@josesimoes josesimoes added the Type: dependencies Pull requests that update a dependency file(s) or version label Nov 13, 2018
@josesimoes
Copy link
Member

josesimoes commented Nov 13, 2018

@sharmavishnu could you please rebase your branch on "upstream/develop" so this PR isn't showing all these unrelated commits?

Remember to pull the changes first as I've just updated it with the latest changes from develop.

Copy link
Member

@josesimoes josesimoes left a comment

Choose a reason for hiding this comment

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

Almost forgot: please update the I2c assembly declaration on the FreeRTOS\ESP32 target.

sharmavishnu and others added 5 commits November 13, 2018 19:52
NativeInit method was getting called everytime I2cDevice.FromId method was called. This resulted in I2C sub-system initialization every time a new I2C device started using a bus. Fixed to ensure that bus is initialized only the first time
@sharmavishnu
Copy link
Contributor Author

  1. I have rebased my fork with the upstream. I still see those older commit messages. Can you point me to the right reading material on the internet to fix this
  2. I have pushed same changes to ESP32, but do not have a board with me to check.

@josesimoes josesimoes changed the base branch from develop to PR-981 November 14, 2018 08:55
@josesimoes
Copy link
Member

@sharmavishnu you are submitting the PR from your develop branch. That causes all sorts of issues when updating from upstream.
Lacking a better (or detailed tutorial) suggest that you follow our contribution workflow.

Basically:

  1. Leave develop branch alone and use it to update from upstream. This is you safety net in case you need to get back, revert or start a new branch.
  2. Work from and submit PRs from features branches. You can isolate work, pull from upstream-develop, rebase and keep a clean history.

@josesimoes
Copy link
Member

I'm opening a branch for this PR to fix the commits and remove the changes.txt file and merge from there.

@josesimoes josesimoes merged commit 566c325 into nanoframework:PR-981 Nov 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: dependencies Pull requests that update a dependency file(s) or version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants