Stability dependent from freqOffset starting values

dwarning commented 2 years ago

Following scenarios: Used Teensy4.1 board and actual master branch.

CC1101Driver.cpp with the original freqOffset 65, 76 Made an Freq Calibration (Menu 6) and got -76kHz offset Checked averageFreqEst in UpdateFreqOffset() with 79kHz Following working has absolute stable values on display over long time

CC1101Driver.cpp with the freqOffset 18, 26, which fits my CC1101 board. Made an Freq Calibration (Menu 6) and got 1kHz offset Checked averageFreqEst in UpdateFreqOffset() with 79kHz too Following working show unstable values on display and breaks over minutes

Also the question arise why UpdateFreqOffset() writes same value into FSCTRL0.

Rodemfr commented 2 years ago

Damn, I merged too quickly in master... I will investigate this. Thanks for testing !

Rodemfr commented 2 years ago

By the way, I also get 79 as offset with my board. It looks like this value is a constant which is not what we think it is. I will temporarily disable the algorithm in the master branch.

dwarning commented 2 years ago

Please read the Design Note 015 - attached.

They say:

_Note that the receiver software must apply the accumulated value to [FSCTRL0.FREQOFF], not just the instant [FREQEST.FREQOFFEST] value.

You are doing an averaging and then give it direct into the register. Don't know why they want have the accumulated (old + new) value. Perhaps the 2'complement or a fast internal register operation.

DN015_Permanent_Frequency_Offset_Compensation.pdf

Rodemfr commented 2 years ago

You are right. It is also mentioned that : It is also important that the CC11xx/CC25xx is in IDLE state when reading [FREQEST.FREQOFF_EST], to ensure that the [FREQEST.FREQOFF_EST] value links to the last received RF packet.

I'm currently not reading FREQEST in idle mode. This might explain the issue...

dwarning commented 2 years ago

Yes - shifting the Update function 10 lines higher brings other values. But don't know if we are there in idle mode. Used accumulated values for writing to the register. Also I think we have to use int8 variables for that - not int32.

`void CC1101Driver::UpdateFreqOffset() { int8_t freqEst, freqEst_old=0; int32_t averageFreqEst;

// Read latest frequency offset estimation and store it in averaging array
freqEst = SpiReadReg(CC1101_FREQEST);
freqEstArray[freqEstArrayIndex++] = freqEst;

// Handle array index loop
if (freqEstArrayIndex >= FREQ_ESTIMATION_ARRAY_SIZE)
{
    freqEstArrayIndex = 0;
    freqEstArrayFilled = true;
}

// Only calculate averaged frequency estimator once array has been entirely filled once
if (freqEstArrayFilled)
{
    // Average frequency offset
    averageFreqEst = 0;
    for (int i = 0; i < FREQ_ESTIMATION_ARRAY_SIZE; i++)
    {
        averageFreqEst += freqEstArray[i];
    }
    averageFreqEst /= FREQ_ESTIMATION_ARRAY_SIZE;
    freqEst = averageFreqEst; // signed 8 bit
    freqEst += freqEst_old; // cummulated
    freqEst_old = freqEst;
    // Write it
    SpiWriteReg(CC1101_FSCTRL0, freqEst);
}

}`

dwarning commented 2 years ago

Sorry, the scope of freqEst_old in the code above was wrong. Made few experiments by calling Update direct after ReadRxFifo() in the hope of idle state. But no success, the variations are to large which are written to the FSCTRL0 register. Communication then breaks.

Rodemfr commented 2 years ago

I think I found the issue by looking to the code from davidwker : FREQEST is a status register not a standard register. It must be read with SpiReadStatus function, not SpiReadreg function !

Rodemfr commented 2 years ago

It seems to work now, values are much more as expected. I'm now accumulating the value as per DN015 app note and it works. I let a debug print to control the offset value. I will remove it when merging into the master branch. I observed pretty different offset values for my two devices. This raises the following interesting question : which frequency should we track : The average frequency of all devices or Master's frequency only ? The current calibration algorithm (menu 6) tracks master's frequency only. I have the feeling that we should do the same with FREQEST algo.

dwarning commented 2 years ago

OK, now it work also for my different freqOffset868 values and looks plausible.Two remarks:

I would save the SPI traffic for the old value register access by using one int8 variable with scope to CC1101Driver. (As I made it in my post before, but wrong local scope.) I mean the SpiWriteReg(CC1101_FSCTRL0, newFreqOff) expect an byte and not int32. Don't know if a cast is sufficient. Should not compiler or uC dependent.

About your FOCCFG setting I must think about. You limited the variation to 1/8 of BW (250kHz).

I see that the algorithm also work in Calibration phase, wanted? Can follow in some trouble?

I agree that the master should be the reference.

dwarning commented 2 years ago

How do you think about this code? `int8_t oldFreqOff=0; void CC1101Driver::UpdateFreqOffset() { int8_t freqEst, newFreqOff; int32_t averageFreqEst;

// Read latest frequency offset estimation and store it in averaging array
freqEst = SpiReadStatus(CC1101_FREQEST);
freqEstArray[freqEstArrayIndex++] = freqEst;

// Handle array index loop
if (freqEstArrayIndex >= FREQ_ESTIMATION_ARRAY_SIZE)
{
    freqEstArrayIndex = 0;
    // Calculate average frequency offset
    averageFreqEst = 0;
    for (int i = 0; i < FREQ_ESTIMATION_ARRAY_SIZE; i++)
    {
        averageFreqEst += freqEstArray[i];
    }
    averageFreqEst /= FREQ_ESTIMATION_ARRAY_SIZE;
    // Update offset
    newFreqOff = averageFreqEst + oldFreqOff;
    if (newFreqOff >= 127) newFreqOff = 127;
    if (newFreqOff <= -128) newFreqOff = -128;
    oldFreqOff = newFreqOff;
    SpiWriteReg(CC1101_FSCTRL0, newFreqOff);
}

} ` The register expect 2'complement for an int8 and get it. Everything OK with your code. I changed the assignment for the old FreqOff to prevent register access.

dwarning commented 2 years ago

Again my question: I see that the algorithm also work in Calibration phase, wanted? Can follow in some trouble?

I would prefer to switch off.

There is an double entry into your CC1101Driver.cpp:

SpiWriteReg(CC1101_FOCCFG, 0x16); SpiWriteReg(CC1101_FOCCFG, 0x1B);

I would prefer not aggressive PID parameter like 0x16.

One question to your new freqEst limiting: If someone starts with worse freq parameter in FSCTRL0 and made no calibration before, can it be that the communication stuck's because it needs too long time for correction phase?

Rodemfr commented 2 years ago

You are right, I switched back PID to 0x16 in my local code.

The freqEst limiting should not prevent the system to work, even with a poor initial FSCTRL0. It will only slow down frequency convergence. The role of this limiter is to decrease frequency jitter. I noticed that once the frequency is established, we still have some significant FREQEST values (up to +-6). So to avoid averaging on very long windows, I'm just limit the offset to +-1.

You are right for RF calibration, I must only disable the algorithm during that phase !

Note that I'm facing a strange issue : since I added the freq offset algo, I face crashes of the SW. Sometimes after 5-10 minutes, sometimes after hours. It seems not directly related to the algorithm itlsef but to the changes in timings at the end of GDO0 interrupt. I suspect that there is a serious timing bug somewhere in the SW which can cause these crashes and which is revealed by this new piece of code. The problem I have is that without good debugging tools it is very difficult to debug. So I'm now investigating a way to enhance my debug capabilities with tools like this one : https://github.com/ftrias/TeensyDebug Until I find this bug, I will not merge the branch in master.

dwarning commented 2 years ago

On which board you are facing the issues? Both, 3.5 and 4.0?

Before your last changes (limiting etc.) I made an extreme test with my 4.1 board: My freqOffset is 18, 26 FreqCal gave -0.5kHz (~1ppm)! Then cooled down the display to -10′C Observed freqEst started 0, next 23 and rised to 24 after 20min and stay there after 3h (~25°C). I think the 35degC temp diff has not sufficient impact to the display freq deviations - perhaps good crystal.

dwarning commented 2 years ago

Made an quick check with 3.6 board and actual frack_tracking branch. Display was stable over 40 minutes. EDIT: Same result with my 4.1 board - 4 hours continuous work.

Rodemfr commented 2 years ago

Yesterday I burnt my Teensy 3.5 setup by mistaking power supply voltage (13V instead of 5V...). My M8N died in the battle. I switched to to Teensy 4.0 with LSM303 but without M8N. Since this switch, I have no more freeze problems on master. It runs for hours. The freq_tracking branch severely crashes however, immediately after boot. This will help investigations. I will soon go for summer holydays, I will likely not answer for several weeks.

dwarning commented 2 years ago

My God! Hope it is not the end of the project. All the modules are seldom now and the prices rises because the Chinese deliver not anymore. You see I closed an issue and open new one. If you have nothing against I will open issues for new results, bugs or whatever. I am not expecting that you will answer in you holidays. Perhaps after your vacation. Last week I made an one day trip (4h sailing and then 6h transfer over lakes and channels). I used my old config (without message split and using elechouse cc1101 library). With serial wifi and openCPN I tracked the entire trip over 30km without any break. I wish you an nice sailing trip. Ship ahoy!

Rodemfr commented 2 years ago

No risk of stopping the project. My setup is running well with Teensy 4.0. I ordered a new GPS. I didn't find a M8N but a M8M (without flash). This should do the job thanks to j-lang's init code. Agree with issue usage. That's good to track remaining issue or possible features.

davidwkerr commented 2 years ago

Sorry to hear about burning up electronics! Sounds a good time to go on a sailing holiday.

I have been rather busy with other things and was also very sick with RSV virus but okay now and catching up on developments.

The MicronettoNMEA code has been running here without stop or glitch for about TWO MONTHS!!!

I did a small write up of my project on the YBW forum. I previously tried to send Rodemfr the photos but GitHub dropped them:

https://forums.ybw.com/index.php?threads/raymarines-micronet.539500/page-10

Rodemfr commented 2 years ago

Frequency tracking has finally been merged to master. It works well.

dwarning commented 2 years ago

This problem (see my opener post 20.07.) still exist. I can't use your coefficients - have to insert my coefficients (18,26) for frequency offset. `byte freqOffset868[2] =

//{ 65, 76 };

{ 18, 26 }; ` If using your coeffs the display will connect after long time or never,

Rodemfr commented 2 years ago

Damn ! I will quickly disable it in master branch. I will first solve the easy issue on remaining warnings and then come back to this one. I will have to dive in datasheet to understand all these values...

dwarning commented 2 years ago

What is happen on your setup, if you using my coefs 18, 26? Do you get quick display values? Should the freq tracking approach find correct values? Only to understand.

Rodemfr commented 2 years ago

With 18,26, it works. Where do you get these values from ? I took the table from ELECHOUSE's driver without actually knowing what is behind...

dwarning commented 2 years ago

The values from ELECHOUSE driver are wrong.

I use the coefficients 18,26 without any change later and have -1ppm deviation from 869,84MHz. This is since beginning of our project. Another cc1101 board shows 17, 20 but can work with 18,26 very well.

Am 05.11.22 um 19:35 schrieb Rodemfr:

With 18,26, it works. Where do you get these values from ? I took the table from ELECHOUSE's driver without actually knowing what is behind...

— Reply to this email directly, view it on GitHub https://github.com/Rodemfr/MicronetToNMEA/issues/14#issuecomment-1304607723, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEMWNTF24SHYDS23O3OKLTLWG2SFZANCNFSM54BQSUPQ. You are receiving this because you modified the open/close state.Message ID: @.***>

Rodemfr commented 2 years ago

I'm reading the datasheet and I'm puzzled by ELECHOUSE's code. My understanding is the following : Setting the frequency should only involve FREQ0, FREQ1 & FREQ2 registers, not FREQOFF. FREQ0/1/2 offer a sub kHz resolution while FREQOFF has a coarser resolution (about 1.5kHz). I understand, without beeing sure, that FREQOFF is only there to compensate for temperature or voltage variations. So I would expect :

AUTOCAL to be set to OFF (I understand it is not required with our frequency tracking algorithm)
FREQOFF to be set to 0 before calibration.
Enable our frequency tracking algorithm to set FREQOFF on a regular basis to follow master's frequency

I will try this now...

Rodemfr commented 2 years ago

I think I'm in the good direction : I created a new branch "debug_freq_tracking" to test my "theory". I'm now setting FREQOFF to 0 when programming FREQ0/1/2. It seems to work well. Note that you have to redo RF calibration which will off course give different values from what you had before.

There are still things I haven't well understood : disabling autocal does not work, while I was expecting it to be OK and there are still some part of datasheet I have to dig.

Tell me if this version works better for you.

Rodemfr commented 2 years ago

Ok, found the issue, when I disable autocal, I need to manually trigger PLL calibration after setting the frequency, that's mandatory.

dwarning commented 2 years ago

I made a first check with debug_freq_tracking branch with the mods from today. It is OK - the display shows quick values, 5m distance rf quality is around 7 (very good), a freq check show 38kHz deviation. Last one is not critical, whenever I can't believe.

Can you wait with merge to master? We have no hurry, master is working for you and for me. I want check this large change next week more deeper and come back with a concluded comment.

Rodemfr commented 2 years ago

Sure, I will wait. Thanks for testing this. I'm now more confident the frequency is properly programmed.

dwarning commented 2 years ago

I made few tests with the debug_freq_tracking branch.

Can't understand CC1101Driver::Calibrate function. She is called in SetFrequency but will write anytime 0 to the register.
UpdateFrequency is also called in RF Calibration Menu. This we have had some time before. I think this will be chaotic if listen to displays frequency and try to change PLL register between.
If I setup in my old approach the FSCTRL0 register with a value of around 20 and then never touch this I got with all cc boards I have 1 .. 5 ppm's deviation from 869.84MHz and very good reception.
In the new approach FSCTRL0 is set to 0 in startup phase. If I make then RF calibration I ends with 42ppm or 35kHz deviation. I can't trust it because I measured my both Raymarine equipment and see clear our Micronet frequency.
In may opinion no frequency tracking is needed, the equipment is very stable over temperature and voltage. A configurable choice is preferable.

Rodemfr commented 2 years ago

Here are my answers but be careful that I'm all but an expert in PLL and RF programming. They need to be taken with caution:

Can't understand CC1101Driver::Calibrate function. She is called in SetFrequency but will write anytime 0 to the register.

I think ELECHOUSE driver had a error/bug based on a bad understanding of FREQOFF register. I read the datasheet and my conclusion is that FREQOFF register is not the one to be used for PLL calibration. PLL calibration is a required internal procedure which I don't know exactly what it is doing but which will result in enabling the PLL output. I also don't know against which source is the PLL calibrated. The result of this calibration is not written in FREQOFF, it is probably stored internally to CC1101. However, it is clearly said that FREQOFF register must not be touched by SW while calibrating, else calibration will be disturbed. FREQOFF can be used after PLL calibration to "move" the frequency for any possible need. In our case we use it to follow master's frequency and compensate for any source of frequency drift.

UpdateFrequency is also called in RF Calibration Menu. This we have had some time before. I think this will be chaotic if listen to displays frequency and try to change PLL register between.

You are right, that's a bug. I must call RfDriver.DisableFrequencyTracking before calibrating RF frequency.

If I setup in my old approach the FSCTRL0 register with a value of around 20 and then never touch this I got with all cc boards I have 1 .. 5 ppm's deviation from 869.84MHz and very good reception.

If my first answer is correct, setting FSCTRL0(FREQOFF) to 20 just adds a unwanted/uncontrolled offset to initial frequency. But this offset is compensated aither by the initial RF calibration menu, either by the frequency tracking algorithm. According to datasheet 20 in FREQOFF means that you shift frequency by ~30Hz.

In may opinion no frequency tracking is needed, the equipment is very stable over temperature and voltage. A configurable choice is preferable.

I think your conclusion is the right one : all the frequency offset found by this algorithm have always been very close to 0 (-1, 0 or +1) in my case. I think I will let the code in, just in case, but will add a configuration switch in BoardConfig.h and disable it by default.

dwarning commented 2 years ago

I think there are two ways to get same result of 869.84MHz, the true freq. we see both from our Raymarine equipment: Your way (and SmartStudio too) set FSCTRL0 to 0 and change the frequency to something else and set the FREQ registers. Using FSCTRL0 only for Compensation on the fly. Disadvantage: Menu 1 show a different Freq. from the true. The elechouse way setting the FSCTRL0 register to a value which brings the frequency and their FREQ registers to the true values and correct Menu 1. In the end both ways bring same result. It is only not easy to understand why we spend so much effort in calibration and tracking if the Menu1 show more as 45ppm deviation. I think there is no mystic internal calibration procedure. Agree with the missing DisableFrequencyTracking switch, but don't forget to switch it on in the end. Agree with the configuration switch.

Rodemfr commented 2 years ago

Reading your post, I suddently realize I misunderstood the ELECHOUSE driver ! The two values between which is interpolated FREQOFF is the calibration for our XTAL !! So following the idea in your latest message we could imagine sweeping FREQOFF value during RF calibration instead of shifting the desired frequency itself. Il like this idea....

dwarning commented 2 years ago

Yes, for one of my boards I have 18,26 (799MHz, 899MHz) interpolated by map function to 24 for the wished freq of 889.84MHz. This offset is added to XTAL freq and results in reference freq for PLL. I would favourize only to use FSCTRL0 register. Possible the timing conditions are better then change FREQ registers.

Rodemfr commented 2 years ago

So, I will merge the branches debug_freq_tracking->reorgonize_code->improve_slave_handling into the master since we have a nice working point. Then, I will create new pull requests to :

Avoid doing 7us wait for every CC1101 command.
Change RF calibration to use FSCTRL0 instead of FREQ0/1/2 registers

Finally, when I will receive my LSM303DLHC, I will fix the remaining problems and we should be ready for a nice new release of MTN.

Rodemfr / MicronetToNMEA

Stability dependent from freqOffset starting values #14