Polymorphism #887

[drmason789]

#887 Use of polymorphism in bluetooth.cpp including taking care of virtual device deletion in bluetoothdevice destructor.

[drmason789]

Overview of changes:

• To remove the code in class bluetooth that looks for the virtual device that is exposed by the device object, the bluetoothdevice class takes care of storage of 1 virtual device. This virtual device is stored using 2 modes, hidden and not, set using the setVirtualDevice function. A hidden virtual device is not exposed using the VirtualDevice() function, a not-hidden device is. A hidden virtual device is for cases such as the Zwift Auto-Resistance Workaround.
• The existing virtual* classes now inherit from virtualdevice, which has some basic shared functionality. More could probably be moved into it.
• The code that previously checked that no virtual device has been assigned, by checking all the fields that store them, has been replaced by a single function hasVirtualDevice().
• The code for each class that creates a virtual device not longer has fields for these devices, and calls setVirtualDevice to set the virtual device once created.
• The bluetooth class no longer has most of the bluetoothdevice subclass fields. These are instead represented by a single bluetoothdevice * field.
• The device() function which used to search that list for the first that's not null, now simply returns the one that's stored.
• The device() template function is used to dynamic cast the existing device to the specified type.

[drmason789]

This changes the behaviour. The loop with the long if statement that detects and connects devices now stops after the first device has been found. This means only 1 device gets connected if there are several. Also, there is code there that checks if a device of a different kind has already been found, previously by checking the device's dedicated field, now by checking if a dynamic_cast of the current device() to the expected type is not null. I'm wondering if I've missed something, because it seems to me that this loop was potentially finding and storing multiple devices, but device() would only return the first one in its defined order.

[drmason789]

Another widespread change, with limited ability to test. Again, it's not a big deal for me if you don't want to merge this. But it does reduce the code and take away some responsibility from the developer (notably storage and destruction of the virtual device).

[cagnulein]

@drmason789 before merging this I have to urgently fix the android 13 compatibilty and release a new production version. So I don't want to put too much gas on the fire. But I will merge this for sure as soon as I will be ready to test it

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[cagnulein]

good friday to you bot :)

[drmason789]

I think I should actually split this into 2: the encapsulation of the virtual devices and the replacement of most of the device fields in class bluetooth. The former I believe to have an immediate benefit and the latter, IMO needs more thought because I think it changes behaviour I think is questionable into a memory leak that is worse.

[drmason789]

I've got the virtual device encapsulation in #994 and PR #995.

[drmason789]

I thought it might make a difference to remove all the now-unused header includes from bluetooth.h and put them in bluetooth.cpp, adding some extra includes elsewhere that this affects. Compared to the previous:

Although it might have built quicker (this time!) the result for this commit was a few bytes bigger:

[drmason789]

I've now removed the encapsulation of the virtual device which I now have in #995.

I'm not sure what's happened with the qmdnsengine submodule yet. I don't think that change is necessary.

[drmason789]

This PR is not ready for merging. See #887.

[cagnulein]

I'm not sure what's happened with the qmdnsengine submodule yet. I don't think that change is necessary.

i updated it. so it must stay on the version of master

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[cagnulein]

pong

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.