bluerange-io / bluerange-mesh

BlueRange Mesh (formerly FruityMesh) - The first completely connection-based open source mesh on top of Bluetooth Low Energy (4.1/5.0 or higher)
https://bluerange.io/
Other
287 stars 109 forks source link

Help syncing timers #120

Closed jjduhamel closed 4 years ago

jjduhamel commented 4 years ago

I'm having trouble getting the timers to sync across nodes in my mesh. When I connect to the node I'm sending an UPDATE_TIMESTAMP packet, and based on the TSYNC logs it appears that the timestamps are getting updated. However, the timers are still firing out of sync with each other.

In the previous version of the firmware, this process approximately worked although some nodes were still out of sync (maybe 80% would be in sync). Since updating to the new firmware all nodes seem to be out of sync.

Is there some new process to sync the timers? My application relies on these being in sync with each other.

mariusheil commented 4 years ago

Hi, we have to look into this issue, but first I have a few questions. When we tested the feature, it worked as expected and also the simulator tests are still passing.

What do you mean by "The timers are still firing out of sync"? The timesync will (as far as I know) not synchronize the timerEvents, as this would lead to weird offsets after the time was synced. There are two different times, one is the globalTimestamp, which is synced as a unix timestamp through the mesh. The other is the application timer, which is starting from 0 once the node boots up and keeps incrementing. The TimerEventHandlers are based on the appTimer.

If you want to fire your timer events in sync, you will probably have to start your own timer once a timesync was received and update that timer once a new timestamp is received. You should wait until the correction packet was received, as this will be more exact than the initial timesync.

jjduhamel commented 4 years ago

Oh interesting. Yes I'm referring to the TimerEventHandler so it sounds like it's expected not to work? Before closing this, could you confirm that there's no way to get the TimerEventHandler to fire in unison across a mesh?

mariusheil commented 4 years ago

I think that the TimerEventHandler is not expected to do this, but I have to check that in the new year. You coud however start your own timer that would be able to do this.

jjduhamel commented 4 years ago

Ok. Please follow up after the new year if it's not too much trouble. In the mean time, I'll look into implementing my own timer as you suggest.

mariusheil commented 4 years ago

One more question: Can you describe your use-case a bit and the timesync accuracy that is necessary? That will better help use find a solution if we need to change anything,

jjduhamel commented 4 years ago

The application is an RGB light which might be used for stage lighting and such. I want to broadcast a sequence of instructions (frames) to each node in the mesh which increment on timer events. Until now, I've been using a dedicated master to broadcast its timer events to the rest of the mesh, but there are drawbacks to this approach. For instance, when the master runs out of battery the entire mesh freezes. It would be preferable if I could rely on each node's timer firing in sync to increment to its next frame.

jjduhamel commented 4 years ago

So after some digging, it seems like this is the place I'd want to re-start my timer?

https://github.com/mwaylabs/fruitymesh/blob/master/src/mesh/Node.cpp#L957

Am I correct that I should initialize the timer in BootModules() in src/mesh/FruityMesh.cpp? I should initially start the timer here as well to handle the case where there's only one node in the mesh (ie no timer sync occurs)?

Finally, I see you recently added some helper functions: CreateTimer, StartTimer, StopTimer; but they're not used anywhere afaik. I'm assuming it'd be preferable to use these methods rather than the StartTimers method which was used previously.

https://github.com/mwaylabs/fruitymesh/blame/master/src/base/FruityHalNrf.cpp#L1749 https://github.com/mwaylabs/fruitymesh/blame/master/src/base/FruityHalNrf.cpp#L1785 https://github.com/mwaylabs/fruitymesh/blame/master/src/base/FruityHalNrf.cpp#L1804

Brotcrunsher commented 4 years ago

Hi, do you have any further progress on this? What are the accuracy requirements that your stage lighting should fulfill? The reason why we are asking for the accuracy requirements again is that we already have systems that are accurate to 0.2 or 1 second.

The use case that you described sounds like it should be implemented via a custom module. Probably this module should somehow store light animation information that tells each node when the light should turn on/off and after the time is synced, no further mesh information should be sent. Every node thereafter "knows" when it should turn on/off without communication with other nodes.

You should almost certainly not change the node module, as its implementation is always subject to change. Upgrading to new fruitymesh versions could be unnecessarily hard for you if you'd perform changes in it. Instead, if you care about if the correction of the time sync has been sent you should call TimeManager::IsTimeCorrected() instead.

Brotcrunsher commented 4 years ago

Hi again, is there any open question remaining?

jjduhamel commented 4 years ago

Hey. Sorry I started working on this again this week. I went the route you suggested, by starting a new timer inside a custom module, but the lights are still slightly out of sync (~200 ms) most of the time.

Here's the method I'm using:

void LedModule::ConfigurationLoadedHandler(...)
{
  ...
  FruityHal::CreateTimer(this->tickTimerId, true, &TickEventHandler);
  FruityHal::StartTimer(this->tickTimerId, 1000);
}

void LedModule::TimerEventHandler(...)
{
  if (!this->timeWasCorrected && GS->timeManager.IsTimeCorrected()) {
    FruityHal::swTimer resetTimerId;

    TimePoint curTime = GS->timeManager.GetTimePoint();
    u32 remainderTicks = ticksPerSecond - curTime.getAdditionalTicks();
    FruityHal::StopTimer(this->tickTimerId);
    FruityHal::CreateTimer(resetTimerId, false, &StartTickTimer);
    app_timer_start((app_timer_id_t)resetTimerId, remainderTicks, this);
  }
}

void LedModule::StartTickTimer(void* ctx)
{ 
    LedModule* self = static_cast<LedModule*>(ctx); 
    FruityHal::StartTimer(self->tickTimerId, 1000);
} 
jjduhamel commented 4 years ago

I'm currently working on a new approach where instead of using the system time to sync the timers, I'll send a global packet to start the timers. Please let me know if you have any other thoughts about why the timers are still out of sync.

Brotcrunsher commented 4 years ago

Hi again! I think there is a problem in the code snipped that you provided. From the Nordic Documentation:

When calling this method on a timer that is already running, the second start operation is ignored.

(see: https://infocenter.nordicsemi.com/index.jsp?topic=%2Fcom.nordic.infocenter.sdk5.v14.0.0%2Fgroup__app__timer.html)

You are starting the tickTimerId twice, once in the ConfigurationLoadedHandler, once in the StartTickTimer function. According to the nordic documentation however, the start inside of the StartTickTimer is completely ignored. Can you try to remove the StartTimer in the ConfigurationLoadedHandler?

Additional side notes:

  1. I am missing some parts of the code snipped to get a full picture of your problem. What's "TickEventHandler"? When is "timeWasCorrected" set to false?
  2. I recommend not calling app_timer_start directly but use the provided HAL functions instead.
Brotcrunsher commented 4 years ago

Hi again, is there any open question remaining?

Brotcrunsher commented 4 years ago

Closing this due to inactivity. Feel free to reopen if further questions arise.