feat(efb): Fix refresh of atc list issue during flight

[ZeroKaa]

Summary of Changes

Reduce the interval between refresh to 1 second

Testing instructions

How to download the PR for QA

Every new commit to this PR will cause a new A32NX artifact to be created, built, and uploaded.

1. Make sure you are signed in to GitHub
2. Click on the Checks tab on the PR
3. On the left side, click on the bottom PR tab
4. Click on the A32NX download link at the bottom of the page

[davidwalschots]

21 commits for a 1 line change? This PR increases the refresh rate. It doesn't decrease it.

[ZeroKaa]

21 commits for a 1 line change? This PR increases the refresh rate. It doesn't decrease it.

21 commits for a 1 line change? This PR increases the refresh rate. It doesn't decrease it.

Hello, indeed ... I update the title ;)

for the commits, I don't know why git add the already merged commits. I did a pull from the branch first

[beheh]

Why have you added lastUpdate as a dependency to this hook? You're not using it.

[ZeroKaa]

the hook executes everytime lastUpdate change.
I had to use this 'hack' otherwise the "currentLatitude" and "currentLongitude" wasn't updated within the setInterval()

[beheh]

In that case you're hiding the real dependencies currentLatitude and currentLongitude. You could refactor the loadAtc to use useCallback instead and make loadAtc a dependency. Or make currentLatitude and currentLongitude a parameter to loadAtc. The proposed change as-is is hacky and not robust at all.

[beheh]

In that case you're hiding the real dependencies currentLatitude and currentLongitude. You could refactor the loadAtc to use useCallback instead and make loadAtc a dependency. Or make currentLatitude and currentLongitude a parameter to loadAtc. The proposed change as-is is hacky and not robust at all.

[ZeroKaa]

@beheh Can you show me an example with a set interval ?
I did my change based on
https://stackoverflow.com/questions/59621744/how-to-use-setinterval-in-react

[ZeroKaa]

@beheh : I updated the code based on your remark. Could you validate this is better now ?
Thanks in advance

[beheh]

Sorry for the delay, one more required change: atisSource is a dependency, so it must be in the hooks array.

There is another general cleanup you could do (but I don't see as a strict requirement): Rather than repeatedly setting apiClient.ATC.getAtc with the same atisSource every 5 seconds when the lat/long changes, you could split that part out into a separate memo which you only redo when atisSource alone changes. Then you simply redo the filtering whenever lat/long changes. In that case you could even completely drop useInterval.

[beheh]

Suggested change

	}, [currentLatitude, currentLongitude]);
	}, [currentLatitude, currentLongitude, atisSource]);

[ZeroKaa]

@beheh

I remove atisSource from dependencies array because the only way to change source is via the settings page and going to settings page and coming back to atc page will reload the component

I call getAtc every 5 minutes because the list of controllers can change in case a controller join or left the Network. So refresh is needed for location changes but also on regular interval.

[beheh]

I remove atisSource from dependencies array because the only way to change source is via the settings page and going to settings page and coming back to atc page will reload the component

If it doesn't change, it's a no-op, so it doesn't hurt having it. Imagine we refactor this component in future and there's another way for the ATIS source to change, maybe a second EFB or an API for a third-party tool or something - at that point we would want this! In all other cases, it does not hurt. Until you're very familiar with hooks I would highly suggest sticking exactly to the rules of hooks, because unfortunately there's a lot of hidden footguns otherwise. Religiously following the rules of hooks protects you from these.

I call getAtc every 5 minutes because the list of controllers can change in case a controller join or left the Network. So refresh is needed for location changes but also on regular interval.

That seems fair. Another way you could do this is following my suggestion, but having a separate counter state variable that you increment every few minutes (and you can reset after a few iterations), so it always triggers an invalidation of the callback.

[ZeroKaa]

I remove atisSource from dependencies array because the only way to change source is via the settings page and going to settings page and coming back to atc page will reload the component

If it doesn't change, it's a no-op, so it doesn't hurt having it. Imagine we refactor this component in future and there's another way for the ATIS source to change, maybe a second EFB or an API for a third-party tool or something - at that point we would want this! In all other cases, it does not hurt. Until you're very familiar with hooks I would highly suggest sticking exactly to the rules of hooks, because unfortunately there's a lot of hidden footguns otherwise. Religiously following the rules of hooks protects you from these.

I call getAtc every 5 minutes because the list of controllers can change in case a controller join or left the Network. So refresh is needed for location changes but also on regular interval.

That seems fair. Another way you could do this is following my suggestion, but having a separate counter state variable that you increment every few minutes (and you can reset after a few iterations), so it always triggers an invalidation of the callback.

Make sense indeed. I will add atisSource.

[ZeroKaa]

@beheh All requested changes implemented. thanks