Add spi aberridg #43
Add spi aberridg #43
Conversation
There was a problem hiding this comment.
Hi Andrew (@aberridg),
Sincere thanks for this - this is excellent stuff.
I've requested some changes - but I'm happy to discuss any and all of them.
If you add any new public functions (methods) please do add them to keywords.txt too. The separators in that file must be tabs not spaces.
Very best wishes,
Paul
There was a problem hiding this comment.
Hi Andrew (@aberridg ),
Great stuff - thank you!
I was going to suggest that the spi buffer should be allocated dynamically, but you beat me to it! It's a nice touch - thank you. We have users for whom that permanently-allocated 20 bytes would have been a problem.
Juts one small change to the debug message (see below) and we're ready to merge.
Please add an SPI example when time permits. It doesn't need to be complex, just a copy of one of the simplest PVT examples would be great. Example3 would be perfectly adequate. But please do update the hardware connections comments to match. Users will need to know how many wires to connect (CS, CIPO, COPI, SCK) and that the DSEL jumper needs to be closed. (Please don't use the terms MISO / MOSI even though you will still see those on many of our boards. We are updating to CIPO / COPI on new boards and on any board revisions).
Great to be working with you and thanks again for the time you've devoted to this. We really appreciate it.
Very best wishes,
Paul
| if (spiBuffer == NULL) { | ||
| if ((_printDebug == true) || (_printLimitedDebug == true)) // This is important. Print this if doing limited debugging | ||
| { | ||
| _debugSerial->print(F("process: memory allocation failed for SPI Buffer!")); |
There was a problem hiding this comment.
Please change process to sendSpiCommand.
(In other libraries, I've started using the full class::function name SFE_UBLOX_GNSS::sendSpiCommand just in case you have debug messages turned on in more than one library simultaneously. But no need to do that here.)
There was a problem hiding this comment.
Thanks Paul - done and done :-)
Please also check my other pull request - that one is much more involved and may need (yet more) tweaking! See comments therein.
And my pleasure - I really NEED this lib for my project, so happy to enhance it to meet my needs and to help others.
…ust downwards if you feel necessary.
|
Thank you for the latest commit - it's another nice touch, thank you. I'm going to walk the dogs, will be back in an hour or so. If you're still tweaking and don't want me to merge when I get back, please just say so here. PS. Just a heads up... Once folks get to know that the library supports SPI, someone will want to SparkFun_u-blox_GNSS_Arduino_Library/src/SparkFun_u-blox_GNSS_Arduino_Library.cpp Lines 3401 to 3409 in 929a9ee |
|
Great! No worries - please do go ahead and merge.
OK on push raw data. It's raining here! Trying to finish a job in the
garden.
Andrew
…On Sun, 27 Jun 2021 at 15:05, Paul ***@***.***> wrote:
Thank you for the latest commit - it's another nice touch, thank you. I'm
going to walk the dogs, will be back in an hour or so. If you're still
tweaking and don't want me to merge when I get back, please just say so
here.
PS. Just a heads up... Once folks get to know that the library supports
SPI, someone will want to pushRawData over SPI too. We will, at some
point (not today), need to add SPI here:
https://github.com/sparkfun/SparkFun_u-blox_GNSS_Arduino_Library/blob/929a9ee6a27a34b653a1c42031cba66d395b1598/src/SparkFun_u-blox_GNSS_Arduino_Library.cpp#L3401-L3409
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#43 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHLJITJOU3XYOXHMIFUH73TTU4V2ZANCNFSM47MA54JA>
.
--
*Andrew Berridge - *TIBCO Data Science | *M*: +44 77 89764017 | *E:*
***@***.*** | www.tibco.com
|
|
OOooh! I just remembered a major issue, Paul! It took me absolutely ages to
figure out.
I found that there must be a bug in the UBLOX firmware... at least for my
NEO-M8U.
It doesn't respond to SPI commands unless you only enable UBX as the input
protocol. If UBX+ NMEA or UBX+NMEA+RTCM2/3 then it doesn't accept any
commands via SPI. It won't even allow me to change the input protocol via
SPI.
Do you have a support contact at UBLOX that could look into this? Is it
possible to test it on another NEO-M of some sort?
I've been using U-Center over USB to configure my device. I think the best
we can do right now is have a big disclaimer in the README??
Thanks!
Andrew
…On Sun, 27 Jun 2021 at 15:32, Andrew Berridge ***@***.***> wrote:
Great! No worries - please do go ahead and merge.
OK on push raw data. It's raining here! Trying to finish a job in the
garden.
Andrew
On Sun, 27 Jun 2021 at 15:05, Paul ***@***.***> wrote:
> Thank you for the latest commit - it's another nice touch, thank you. I'm
> going to walk the dogs, will be back in an hour or so. If you're still
> tweaking and don't want me to merge when I get back, please just say so
> here.
>
> PS. Just a heads up... Once folks get to know that the library supports
> SPI, someone will want to pushRawData over SPI too. We will, at some
> point (not today), need to add SPI here:
>
> https://github.com/sparkfun/SparkFun_u-blox_GNSS_Arduino_Library/blob/929a9ee6a27a34b653a1c42031cba66d395b1598/src/SparkFun_u-blox_GNSS_Arduino_Library.cpp#L3401-L3409
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#43 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AHLJITJOU3XYOXHMIFUH73TTU4V2ZANCNFSM47MA54JA>
> .
>
--
*Andrew Berridge - *TIBCO Data Science | *M*: +44 77 89764017 | *E:*
***@***.*** | www.tibco.com
--
*Andrew Berridge - *TIBCO Data Science | *M*: +44 77 89764017 | *E:*
***@***.*** | www.tibco.com
|
|
Here's the public post I made on the subject:
https://portal.u-blox.com/s/feed/0D52p0000AxS74FCQS?t=1624806640652
And I have submitted a support case. No idea if I'll get any response to
the support case!
Andrew
…On Sun, 27 Jun 2021 at 16:00, Andrew Berridge ***@***.***> wrote:
OOooh! I just remembered a major issue, Paul! It took me absolutely ages
to figure out.
I found that there must be a bug in the UBLOX firmware... at least for my
NEO-M8U.
It doesn't respond to SPI commands unless you only enable UBX as the input
protocol. If UBX+ NMEA or UBX+NMEA+RTCM2/3 then it doesn't accept any
commands via SPI. It won't even allow me to change the input protocol via
SPI.
Do you have a support contact at UBLOX that could look into this? Is it
possible to test it on another NEO-M of some sort?
I've been using U-Center over USB to configure my device. I think the best
we can do right now is have a big disclaimer in the README??
Thanks!
Andrew
On Sun, 27 Jun 2021 at 15:32, Andrew Berridge ***@***.***> wrote:
> Great! No worries - please do go ahead and merge.
>
> OK on push raw data. It's raining here! Trying to finish a job in the
> garden.
>
> Andrew
>
> On Sun, 27 Jun 2021 at 15:05, Paul ***@***.***> wrote:
>
>> Thank you for the latest commit - it's another nice touch, thank you.
>> I'm going to walk the dogs, will be back in an hour or so. If you're still
>> tweaking and don't want me to merge when I get back, please just say so
>> here.
>>
>> PS. Just a heads up... Once folks get to know that the library supports
>> SPI, someone will want to pushRawData over SPI too. We will, at some
>> point (not today), need to add SPI here:
>>
>> https://github.com/sparkfun/SparkFun_u-blox_GNSS_Arduino_Library/blob/929a9ee6a27a34b653a1c42031cba66d395b1598/src/SparkFun_u-blox_GNSS_Arduino_Library.cpp#L3401-L3409
>>
>> —
>> You are receiving this because you were mentioned.
>> Reply to this email directly, view it on GitHub
>> <#43 (comment)>,
>> or unsubscribe
>> <https://github.com/notifications/unsubscribe-auth/AHLJITJOU3XYOXHMIFUH73TTU4V2ZANCNFSM47MA54JA>
>> .
>>
>
>
> --
>
> *Andrew Berridge - *TIBCO Data Science | *M*: +44 77 89764017 | *E:*
> ***@***.*** | www.tibco.com
>
>
>
--
*Andrew Berridge - *TIBCO Data Science | *M*: +44 77 89764017 | *E:*
***@***.*** | www.tibco.com
--
*Andrew Berridge - *TIBCO Data Science | *M*: +44 77 89764017 | *E:*
***@***.*** | www.tibco.com
|
|
Thanks again Andrew, |
|
Awesome! On all accounts...
The case number is: CA-133162
Thank you for all your help and support...
My SPI test to test the basic comms... Works if protocol in is UBX. Doesn't
work with any other combination of in protocol over SPI.
#include <ESP32DMASPIMaster.h>
ESP32DMASPI::Master master;
static const uint32_t BUFFER_SIZE = 8;
uint8_t* spi_master_tx_buf;
uint8_t* spi_master_rx_buf;
void set_buffer()
{
spi_master_tx_buf[0] = 0xB5;
spi_master_tx_buf[1] = 0x62;
spi_master_tx_buf[2] = 0x06;
spi_master_tx_buf[3] = 0x08;
spi_master_tx_buf[4] = 0x00;
spi_master_tx_buf[5] = 0x00;
spi_master_tx_buf[6] = 0x0E;
spi_master_tx_buf[7] = 0x30;
for (uint32_t i = 8; i < BUFFER_SIZE; i++)
{
spi_master_tx_buf[i] = i & 0xA1;
}
memset(spi_master_rx_buf, 0, BUFFER_SIZE);
}
void setup()
{
Serial.begin(115200);
// to use DMA buffer, use these methods to allocate buffer
spi_master_tx_buf = master.allocDMABuffer(BUFFER_SIZE);
spi_master_rx_buf = master.allocDMABuffer(BUFFER_SIZE);
set_buffer();
delay(5000);
master.setDataMode(SPI_MODE0); // for DMA, only 1 or 3 is available
// master.setFrequency(SPI_MASTER_FREQ_8M); // too fast for bread
board...
master.setFrequency(1000000);
master.setMaxTransferSize(BUFFER_SIZE);
master.setDMAChannel(1); // 1 or 2 only
master.setQueueSize(1); // transaction queue size
// begin() after setting
// HSPI = CS: 15, CLK: 14, MOSI: 13, MISO: 12
master.begin(); // default SPI is HSPI
}
void loop()
{
// start and wait to complete transaction
master.transfer(spi_master_tx_buf, spi_master_rx_buf, BUFFER_SIZE);
// show received data (if needed)
for (size_t i = 0; i < BUFFER_SIZE; ++i) {
if (spi_master_rx_buf[i] != 0xFF) {
if (spi_master_rx_buf[i] == 0xB5 || spi_master_rx_buf[i] == 0x24)
{
printf("\n");
}
printf("%x ", spi_master_rx_buf[i]);
//printf("%c", spi_master_rx_buf[i]);
}
}
//printf("\n");
delay(1);
}
…On Sun, 27 Jun 2021 at 16:39, Paul ***@***.***> wrote:
Merged #43
<#43>
into release_candidate.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#43 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHLJITLLSJDKKNOJO7RCHJ3TU5A3NANCNFSM47MA54JA>
.
--
*Andrew Berridge - *TIBCO Data Science | *M*: +44 77 89764017 | *E:*
***@***.*** | www.tibco.com
|
|
Hi Andrew (@aberridg ), It has been a successful day! You will see from the latest commits on the release_candidate branch that I have made some changes to your code. The only significant ones are:
The current status is:
I haven't yet got it working on Teensy 3.2. Not sure why. Maybe the faster processor is hammering the SPI bus a bit too hard? I need to work on other things tomorrow, but over the next few days I will try and rebase the release_candidate_v2.1 branch to include the latest changes, and then give that code a go. I think what I might do is add SPI support in v2.0.8 and then release the modified state management code as v2.1.0 after giving it a good shake. Thanks again for all the effort you've put into this, |
|
Hi Paul,
This is all fantastic news! I'm so excited that you fixed the issue with
the device not accepting UBX commands. I feel so silly now ;-)
In one of my tests I tried sending 0xFF to receive bytes from the device,
but it didn't work. I must have changed a few other things, then it
suddenly started. In fact 0x0A was just randomly chosen by me. I forgot to
check if it had some significance! You're right - it must have been
confused by 0x0A is it's carriage return. Oops!
And perfect on making the example more generic. I couldn't test it on any
other platform so didn't want to experiment with something that I couldn't
verify.
Teensy 3.2 - interesting. Not sure why that wouldn't work. I have a huge
amount of experience with the Teensy platform, including developing my own
custom boards using it. I don't have a way of testing SPI withTeensy 3.2.
My other board that uses the NEO-M8-U uses i2c for communication. I'd be
surprised if it was the speed of the processor. After all, the ESP32 runs
at 240Mhz and the Teensy 3.2 runs at 72/96. I would suspect the underlying
SPI library might be more likely the cause of the issue.
Yes - please do give the 2.1 code a good shake! I'm sure there are a few(!)
issues remaining. The one I know of right now is that it's a bit wasteful
on the various packet buffers. It could also be optimised slightly in other
ways too.
I'm happy to keep working on it once you have given it a test and some
feedback (if I get time)! This UBX stuff is part of a much, much larger
hobby project.
Thanks and great to be working with you - your latest message is exciting
for me because it illustrates how working together on open source projects
builds better and better software!
Andrew
…On Mon, 28 Jun 2021 at 18:15, Paul ***@***.***> wrote:
Hi Andrew ***@***.*** <https://github.com/aberridg> ),
It has been a successful day! You will see from the latest commits on the
release_candidate branch that I have made some changes to your code. The
only significant ones are:
- I changed your debug printf's to print's as the ATmega328 does not
support printf.
- I moved the *new* (memory allocation) for the SPI buffer into .begin.
Why? I noticed that you were still initializing the buffer with 0xFF in
there. It seemed more logical to create the buffer, fill it with 0xFF, then
do the isConnected test.
- I changed the 0x0A you were using on COPI for SPI 'read' transfers
to 0xFF. This made a big difference to the reliability. After doing this, I
can start the code reliably on multiple platforms and multiple u-blox
modules using the factory default settings. No need to set SPI-in to UBX
only. I suspect the line feeds were causing confusion for the module
(especially when NMEA-in was still enabled)?
- I made your example a bit more generic, allowing it to run on the
ATmega328 without modification. I also moved it into its own folder within
the examples.
The current status is:
- your code and example work OK on the following modules using factory
defaults:
- NEO-M8U
- NEO-M9N
- ZED-F9P
- and ZOE-M8Q (which I'm really pleased about as that does not have
a USB port for 'back door' configuration)
- your code runs on:
- ATmega328 RedBoard (== Arduino Uno)
- ESP32 Thing Plus
- Artemis ATP (using Apollo3 core v2.1)
I haven't yet got it working on Teensy 3.2. Not sure why. Maybe the faster
processor is hammering the SPI bus a bit too hard?
I need to work on other things tomorrow, but over the next few days I will
try and rebase the release_candidate_v2.1 branch to include the latest
changes, and then give that code a go. I think what I might do is add SPI
support in v2.0.8 and then release the modified state management code as
v2.1.0 after giving it a good shake.
Thanks again for all the effort you've put into this,
Paul
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#43 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHLJITKHQNBA4DZLZS3OTFTTVCU3JANCNFSM47MA54JA>
.
--
*Andrew Berridge - *TIBCO Data Science | *M*: +44 77 89764017 | *E:*
***@***.*** | www.tibco.com
|
|
Hi Andrew, |
Initial add of SPI support to the library.