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 pulseIn() #2538

Merged
merged 1 commit into from
Aug 7, 2022
Merged

Fix pulseIn() #2538

merged 1 commit into from
Aug 7, 2022

Conversation

mikee47
Copy link
Contributor

@mikee47 mikee47 commented Aug 5, 2022

Very old implementation. Update to use CPU clock as timing reference.

Requires testing with real hardware.

@mikee47
Copy link
Contributor Author

mikee47 commented Aug 5, 2022

@SimonBrazell Can you give this PR a try with your sensor, see if it fixes your issues?

@SimonBrazell
Copy link
Contributor

@mikee47 will do!

@slaff slaff added this to the 4.7.0 milestone Aug 5, 2022
@slaff
Copy link
Contributor

slaff commented Aug 6, 2022

@SimonBrazell Any results?

@SimonBrazell
Copy link
Contributor

No sorry, I got busy shortly afterwards, I'll see how it goes this evening.

@SimonBrazell
Copy link
Contributor

SimonBrazell commented Aug 6, 2022

Just tried to build the Ultrasonic_HCSR04 sample but make failed with the following error:

C+ /Users/sbrazell/Developer/Sming/Sming/Core/ArduinoCompat.cpp
In file included from /Users/sbrazell/Developer/Sming/Sming/Core/SmingCore.h:59,
                 from /Users/sbrazell/Developer/Sming/Sming/Core/ArduinoCompat.h:15,
                 from /Users/sbrazell/Developer/Sming/Sming/Core/ArduinoCompat.cpp:13:
/Users/sbrazell/Developer/Sming/Sming/Core/Data/Stream/FileStream.h: In constructor 'FileStream::FileStream(const String&, FileOpenFlags)':
/Users/sbrazell/Developer/Sming/Sming/Core/Data/Stream/FileStream.h:29:92: error: use of deleted function 'FileStream::FileStream() [inherited from IFS::FileStream]'
   29 |  FileStream(const String& fileName, FileOpenFlags openFlags = File::ReadOnly) : FileStream()
      |                                                                                            ^
/Users/sbrazell/Developer/Sming/Sming/Core/Data/Stream/FileStream.h:23:25: note: 'FileStream::FileStream() [inherited from IFS::FileStream]' is implicitly deleted because the default definition would be ill-formed:
   23 |  using IFS::FileStream::FileStream;
      |                         ^~~~~~~~~~
/Users/sbrazell/Developer/Sming/Sming/Core/Data/Stream/FileStream.h:23:25: error: use of deleted function 'IFS::FileStream::FileStream()'
In file included from /Users/sbrazell/Developer/Sming/Sming/Core/Data/Stream/FileStream.h:13,
                 from /Users/sbrazell/Developer/Sming/Sming/Core/SmingCore.h:59,
                 from /Users/sbrazell/Developer/Sming/Sming/Core/ArduinoCompat.h:15,
                 from /Users/sbrazell/Developer/Sming/Sming/Core/ArduinoCompat.cpp:13:
/Users/sbrazell/Developer/Sming/Sming/Core/Data/Stream/IFS/FileStream.h:22:7: note: 'IFS::FileStream::FileStream()' is implicitly deleted because the default definition would be ill-formed:
   22 | class FileStream : public FsBase, public ReadWriteStream
      |       ^~~~~~~~~~
/Users/sbrazell/Developer/Sming/Sming/Core/Data/Stream/IFS/FileStream.h:22:7: error: no matching function for call to 'IFS::FsBase::FsBase()'
In file included from /Users/sbrazell/Developer/Sming/Sming/Components/IFS/src/include/IFS/File.h:22,
                 from /Users/sbrazell/Developer/Sming/Sming/Core/FileSystem.h:19,
                 from /Users/sbrazell/Developer/Sming/Sming/Core/SmingCore.h:22,
                 from /Users/sbrazell/Developer/Sming/Sming/Core/ArduinoCompat.h:15,
                 from /Users/sbrazell/Developer/Sming/Sming/Core/ArduinoCompat.cpp:13:
/Users/sbrazell/Developer/Sming/Sming/Components/IFS/src/include/IFS/FsBase.h:36:2: note: candidate: 'IFS::FsBase::FsBase(IFS::IFileSystem*)'
   36 |  FsBase(IFileSystem* filesys) : fileSystem(FileSystem::cast(filesys))
      |  ^~~~~~
/Users/sbrazell/Developer/Sming/Sming/Components/IFS/src/include/IFS/FsBase.h:36:2: note:   candidate expects 1 argument, 0 provided
/Users/sbrazell/Developer/Sming/Sming/Components/IFS/src/include/IFS/FsBase.h:33:7: note: candidate: 'constexpr IFS::FsBase::FsBase(const IFS::FsBase&)'
   33 | class FsBase
      |       ^~~~~~
/Users/sbrazell/Developer/Sming/Sming/Components/IFS/src/include/IFS/FsBase.h:33:7: note:   candidate expects 1 argument, 0 provided
/Users/sbrazell/Developer/Sming/Sming/Components/IFS/src/include/IFS/FsBase.h:33:7: note: candidate: 'constexpr IFS::FsBase::FsBase(IFS::FsBase&&)'
/Users/sbrazell/Developer/Sming/Sming/Components/IFS/src/include/IFS/FsBase.h:33:7: note:   candidate expects 1 argument, 0 provided
make[2]: *** [Core/ArduinoCompat.o] Error 1
make[1]: *** [Sming-build] Error 2
make: *** [all] Error 2

@mikee47
Copy link
Contributor Author

mikee47 commented Aug 6, 2022

Try make dist-clean first.

@SimonBrazell
Copy link
Contributor

Okay that did it, it appears to be measuring much more accurately now but with the exception of a few 0 readings in a row from time to time:

Screen Shot 2022-08-06 at 22 01 07

@slaff
Copy link
Contributor

slaff commented Aug 7, 2022

with the exception of a few 0 readings

@mikee47 any idea what might cause the 0 readings?

@mikee47
Copy link
Contributor Author

mikee47 commented Aug 7, 2022

with the exception of a few 0 readings

@mikee47 any idea what might cause the 0 readings?

50us echo timeout. Disabling interrupts would likely fix that, but more realistically applications should discard 0 reads.

@slaff
Copy link
Contributor

slaff commented Aug 7, 2022

but more realistically applications should discard 0 reads.

Ok, shall I merge the PR as is?

@mikee47
Copy link
Contributor Author

mikee47 commented Aug 7, 2022

Ok, shall I merge the PR as is?

Yes, looks like pulseIn is doing what it's supposed to.

@slaff slaff merged commit 5d2000a into SmingHub:develop Aug 7, 2022
@slaff slaff removed the 3 - Review label Aug 7, 2022
@mikee47 mikee47 deleted the fix/pulsein branch August 8, 2022 05:15
@slaff slaff mentioned this pull request Nov 8, 2022
5 tasks
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.

3 participants