lorabasics / basicstation

LoRa Basics™ Station - The LoRaWAN Gateway Software
https://doc.sm.tc/station
Other
340 stars 178 forks source link

Idea about implementing the Fine timestamp functionality in a Basic Station (Corecell design and SX1303 or SX1302 chip) #177

Open LouneCode opened 1 year ago

LouneCode commented 1 year ago

Hi,

I have an idea how to enable the Fine timestamp functionality on Basic Station. The idea only applies to the Corecell design (std, stdn variants) and the latest chips SX1303 and SX1302. An example of the concept can be found in the LouneCode/gls-basicstation repository.

It would be nice if someone from Semtech would review the code changes and give feedback on this proposal. The easiest way to investigate the required code changes is to look differences of this patcht.

Code changes shortly:

Cheers LouneCode - Only husky in the village

reissjason commented 1 year ago

Also interested having fine-timestamps implemented for SX1303. I was expecting to see this added last year.

LouneCode commented 1 year ago

I've been testing the proposed basic station code with the fine timestamp changes (+ ChirpStack) for couple weeks with no problems;)

brocaar commented 1 year ago

@beitler could you provide any feedback on this issue?

LouneCode commented 1 year ago

Radio silence continues... Is this repository still active? @beitler or anyone there, could you please give some feedback on this matter.

beitler commented 1 year ago

Hi @LouneCode , I'm sorry for the silence. I saw your patch and I think you did a great job there. Thank you very much for your contribution. There is just one place where we need to be more careful:

https://github.com/LouneCode/gls-basicstation/blob/57df4123f4ae6594ffcbde0fba3fe51c849c8930/builder/GLS_v2.0.6.1.patch#L139

It's an interesting idea to replace the sub-second part of the rxtime with the precise timestamp. However, generally rt_getUTC() cannot be assumed to be synchronized to GPST. Also, at the time rt_getUTC() is called, it may have advanced a second (e.g. due to rx jobs being queued), such that the sub-second part will not match the full second. Note that for geolocation purposes, the absolute receive time is not necessary. So, I'm wondering, do you have a particular use case in mind for this behavior?

@brocaar the fts field is currently not documented in the LoRa Basics Station LNS protocol. However, we will most probably add it with the next release which brings fine time stamping support for the corecell platform. The fts field already existed in the protocol in order to support the less common DSP-based 'V2' reference designs.

LouneCode commented 1 year ago

Thank You @beitler for your feedback.

"So, I'm wondering, do you have a particular use case in mind for this behavior?" : No, I don't have a specific use case for this behavior. in the patch, UTC part of the fts element has been more or less informative part of the fine timestamp. I have only use it for investigating this solution. I am aware of this GPTS and UTC timestamp synchronization issue. In practice, you only need a sub-second part (ns) to calculate geolocation information.

Do you have any other idea for this UTC part of the fine timestamp. Perhaps there no need to use UTC part at all?

LouneCode commented 1 year ago

Sorry, I messed a things in the previous comment. Please forget this comment:) Do you have any other idea for this UTC part of the fine timestamp. Perhaps there no need to use UTC part at all?

You mean the following patch code in src/s2e.c file and the rxtime field :

 @@ -172,6 +189,7 @@ void s2e_flushRxjobs (s2ctx_t* s2ctx) {
             reftime = s2ctx->muxtime +
                 ts_normalizeTimespanMCU(rt_getTime()-s2ctx->reftime) / 1e6;
         }
+
         uj_encKVn(&sendbuf,
                   "RefTime",  'T', reftime,
                   "DR",       'i', j->dr,
@@ -183,7 +201,8 @@ void s2e_flushRxjobs (s2ctx_t* s2ctx) {
                   /**/ "fts",     'i', j->fts,
                   /**/ "rssi",    'i', -(s4_t)j->rssi,
                   /**/ "snr",     'g', j->snr/4.0,
-                  /**/ "rxtime",  'T', rt_getUTC()/1e6,
+                  // Append fractal part of the fine timestap (fts) to rxtime if exsist.
+                  /**/ "rxtime",  'T', j->fts > -1 ? (sL_t)(rt_getUTC()/1e6) + (double)j->fts/1e9 : rt_getUTC()/1e6,
                   "}",

This fine timestamp fractal part in the rxtime field has been there for testing purposes only. I Think this change is not needed and can be reverted back as follows.

/**/ "rxtime",  'T', rt_getUTC()/1e6,
svsh1990 commented 9 months ago

It's so needed! When?