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 esp32 support with CRC handling #82

Merged
merged 9 commits into from
Sep 21, 2023

Conversation

peterj43
Copy link
Contributor

The changes here resolve a problem encountered when using the "preferences" library. When updating the setting stored in the Preferences I was encountering a Core panic as follows:-
"esp32 Guru Meditation Error: Core 1 panic'ed (Cache disabled but cached memory region accessed)"
This occurred when acknowledging the SDI-12 command.
Note: CRC handling is also included.

@neilh10
Copy link

neilh10 commented Jul 13, 2021

Well fascinating. Good 2 see.
I haven't used ESP32 much, a quick comment is that I would hope the
USE_CRC
could be given a better name space,
SDI12_USE_CRC or even ENVIRODIY_SDI12_USE_CRC

Possibly also USE_INSTRUCTION_RAM
could be given ESPFAMILY_USE_INSTRUCTION_RAM

I hope to look at the code in more detail later, as the SDI12 is core to an interface to the LT500 I use, and I'm under a time crunch to look at it now.
I wonder if something could be said about the reference implementation, testing, and what target instruments this is tested against. Possibly as part of #79

@neilh10
Copy link

neilh10 commented Jul 14, 2021

@peterj43 , thanks for the posting it.
It would be nice to see this accepted as a branch that can be tried on real instruments.
I would be able to try it with the LT500.
The code that I can see is just for the CRC calculation, to use a 'C' would be appended to the command and then processing the three characters returned, would be needed.

@peterj43
Copy link
Contributor Author

Happy to make the suggested changes.

@neilh10
Copy link

neilh10 commented Jul 15, 2021

@peterj43 thanks for the update. I've cloned your repo so have a copy and hope to test the CRC with the LT500 that I use at some point. I'm mostly a follower of the EnviroDIY::xx , and really appreciate they have open sourced this project.
I think they are mostly grant driven projects that they do - but I don't have any further insights other than the ones online and appreciate that SRGDamia often is extending the lib.
I do test and discuss the testing on EnviroDIY. https://www.envirodiy.org/topic/stability-testing-how-to-do-it/
Testing SDI12/Mayfly mega1284 is particularly challenging with an amazing uart emulation it has, so appreciate any insights you have for the ways you test. :)

@peterj43
Copy link
Contributor Author

@neilh10 SDI-12 isn't the connection technology of choice for our sensors but it had been suggested the we off the option. Most of our installations use SigFox. We looked to use the ESP32 as it seemed to tick a lot of boxes for us, as we have prior experience of the part and we could have a variety of connection options. Though we currently have no application where the CRC is being used I felt it was possible that a customer may request it being supported. Obviously I have done some testing, using examples from the SDI-12 Spec, but I have not tested it against a commercially available logger. I suppose I should look to a the capability to the logger example but that's not really a priority for me at present.

https://github.com/peterj43/Arduino-SDI-12 into Add-ESP32-Support

# Conflicts:
#	src/SDI12.cpp
#	src/SDI12.h
#	src/SDI12_boards.cpp
#	src/SDI12_boards.h
@neilh10
Copy link

neilh10 commented Jul 16, 2021

@peterj43 ahh interesting, if I understand you then your are looking at the ESP32 as the test "host" to verify your sensor has implemented the SDI-12 responses.

@SRGDamia1 SRGDamia1 changed the base branch from master to crc September 21, 2023 18:28
@SRGDamia1 SRGDamia1 changed the base branch from crc to master September 21, 2023 18:41
@SRGDamia1 SRGDamia1 changed the base branch from master to CRC September 21, 2023 18:45
@SRGDamia1 SRGDamia1 merged commit 7abc730 into EnviroDIY:CRC Sep 21, 2023
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