@aknrdureegaesr how do you want to handle this? There is already a 2.4 tag at that commit, it just hasn't been branched (yet). I intended to create the 2.4 branch from that commit this evening yet to store a "snapshot" in history where the codebase has been thoroughly thrashed and is as bug-free as we can make it.

So do you want this to be a WIP and label it as such? Or would you rather merge it to master as-is and fix any bugs later? I'd like to have @bazineta take a look at this. He did the port to C++ and is most familiar with this part of the code. I'm not thoroughly convinced your fix achieves the desired result since in my testing I haven't been able to duplicate any timing issues, or noticed any failed decodes on the receiving end. An HB or CQ is always transmitted in one frame here no matter what speed mode is used, and replies always come on the very next frame.

There's a lot to digest here. A couple of initial comments.

1. I question the use of the volatile qualifier in the modulator; use of volatile outside of in IRQ handling context is somewhat unusual in modern C++, and I would either expect a very clear rationale for it to be explained in relevant commentary, or a more current solution to be used.
2. The singleton pattern used in the DriftingDateTime class likewise reads like Java, and is a bit unusual in modern C++. If a singleton implementation is indeed required here, I would suggest investigation of more current techniques, e.g., use of a Meyers singleton if a singleton is required, and perhaps hiding this implementation detail via namespace facade, so that consumers to not have to be aware of it.
3. However, with respect to (2), I suspect that given the small integral size of the value being so protected, use of std::atomic<> or a lock-free implementation might be the simpler solution.

To add more context to observation (3), for small integral-sized values, a pattern like the following is simple and efficient; an excerpt from some of my own stuff (the time_point used here is just what my stuff happens to use; any integral value is in general fine as a std::atomic<> now):

Class member, private:

std::atomic<std::chrono::system_clock::time_point> _last;

Accessor and mutator, public:

[[nodiscard]] auto
last() const noexcept
{
            // Readers only need the timestamp value for freshness
            // checks/persistence. Writer uses release-store; readers
            // use relaxed-load to avoid fences.
            
            return _last.load(std::memory_order_relaxed);
}
void
setLast(std::chrono::system_clock::time_point const last) noexcept
{
            // Publish the updated timestamp for other threads promptly;
            // no pairing acquire is required by current readers.
            
            _last.store(ltd, std::memory_order_release);
}

This has the desired effect of the protective singleton without all the ceremony; it might be a good pattern here. While I've excerpted from what is a class in my implementation, this works equally well at namespace scope when the mission is just to have a single thread-safe global value.

I intended to create the 2.4 branch from that commit this evening

Please do so!

I'd suggest: Start a 2.4.0 release branch from that commit. That would be a good place to develop whatever bits are missing to run an actual release, possibly including "how to release" documentation.

The main branch can continue towards 2.4.1 at the same time.

After the 2.4.0 release is a reality, we can later rebase a copy of the 2.4.0 release branch to either become a part of the main branch or of a new 2.4.1 release branch.

This does not yet even work. Can no longer type in messages. Need to fix that. So I decidedly object to this being merged as it presently sits.

My own objection has been addressed:

This does not yet even work.

For all I know, it does now. I'll try a few QSOs tomorrow.

To add more context to observation (3), for small integral-sized values, a pattern like the following is simple

I didn't know what std::atomic is and had to look it up. (Most of my professional C++ coding was back in the early to middle 90s, when the STL was still young and our project management didn't allow us developers to use it 🤦.)

The singleton pattern used in the DriftingDateTime class likewise reads like Java, and is a bit unusual in modern C++.

Guilty as charged. (After moving away from C++, I spent the next 10 or 15 years or so as a Java developer.)

If a singleton implementation is indeed required here, I would suggest investigation of more current techniques, e.g., use of a Meyers singleton if a singleton is required, and perhaps hiding this implementation detail via namespace facade, so that consumers to not have to be aware of it.

Why is this a singleton?

I want a QObject so it can tie in with the signal / slot mechanism. That is already used: If the drift changes, the TxLoop get that information via signal / slot and re-adjusts its timer so it comes out at the appointed time, taking the new drift into account. I didn't want to have several clocks around with various different drifts, that would be outright dangerous. It would in theory be possible to have several clock objects with one common (static) drift member. But then, how would object A send signals if that member had been changed through object B? A can of worm we don't need. Let's go singleton. It fits the bill.

Java-esce or using the power of static?

Doing it the Java way was not my not knowing how to use static, but a result of me being a bit paranoid. Could it be the system would legally be allowed to instantiate a static singleton earlier than when it is actually accessed for the first time? My implementation sees to that this doesn't happen. Why do I think about this? I am simply not sure how Qt reacts when an object is constructed at a time before the thread has set up a Qt event loop, an object that object later joins the event sending and receiving fun (which is, as said, the whole point of having an object in the first place).

I'm pretty sure it doesn't hurt to construct early.

But only "pretty sure". Could it happen that the UI thread isn't the initial thread that starts the whole program? If I'm not sure, do I need to move such a statically generated object from one thread to another? If so, when is the opportune time to do this?

And I even boast that lazyness a virtue: It lifts the burden to think through the same issues again from anybody who comes to this code after me.

We are application programmers. In my opinon, we should try to avoid subtleties if comprehensible "pedestrian" code gets the job done quite as well. My "Java way" is robust, as far as I can tell, and it fits the bill. Any performance hit will be quite negligible.

Finally, why QMutex and not std::atomic?

In a Qt project, using QMutex and QMutexLocker is doing things the Qt way. On the other hand, std::atomic is a C++ way of doing similar things (though it is more restricted). Now this is about offering services to code that operate in the Qt universe. The DriftingDateTime is a thin layer around QDateTime. Using QMutex and QMutexLocker makes that layer thinner.

I hope you enjoy this discussion, @bazineta .

What do you think? Can we count your objection as answered, without me changing the DriftingDateTime code?

I question the use of the volatile qualifier in the modulator

The method isIdle() is used from MainWindow, which lives in another thread. As, at the end of the day, an enum is a short integer, I thought making it volatile was the cheapest solution to making access thread-safe. What would you suggest?

Later edit: volatile is not what I thought it was. I need to change the code to make it thread-safe.

Later edit: volatile is not what I thought it was. I need to change the code to make it thread-safe.

Yes....the meaning of the term has changed since 2011, and now, it means 'data race here' ;).

Using std::atomic<> is probably your short path given the use case.

So, thanks to @bazineta for the review comments! I learned a thing or two in the process!

In the end, I decided to use plain store and load with the standard arguments. I considered paying the price to document why we would accept the (low) risk of the modulator writing a new state and the main window reading an older one, and/or having subsequent readers of the code wonder about it. My judgement is that this price is not worth paying for a (wild guess) few hundred nanoseconds runtime improvement every 100 ms or so.

If I didn't overlook anything, I have by now reacted to all objections. No reviewer has as of yet uttered the coveted "lgtm". So this PR is again up in limbo, like a new PR nobody has yet looked at.

It's been looked at plenty. It builds, seems to run without issues (on MacOS 26). I'd say if @bazineta has no real issues with it, go ahead and merge it. While it won't create a conflict anyway, I'd like to bump master to 2.4.1-dev as soon as this gets merged.

LGTM.

Merged. Thanks!