rgleason / watchdog_pi

watchdog plugin for opencpn
GNU General Public License v3.0
1 stars 10 forks source link

API Update: Improved PIM Version numbering requires updated API 118 - Yes, close. #70

Closed Corsair-63 closed 10 months ago

Corsair-63 commented 1 year ago

Windows 11 X64 OCPN version 5.8.4-0

updated catalog beta version and found version WD 2.4.74, installed correctly. coming back to master catalog, click on update and shown version installed 2.4.70 (first issue) only shown in the plugin tab the correct version if the catalog is set to Beta. gone to C:\Users\XXXX\AppData\Local\opencpn\plugins and checked that WD files has today date, so must be 2.4.74 version.

I don't know if I'm doing something wrong:

rgleason commented 1 year ago

Plugin installation.

  1. Update catalog master or beta.
  2. Select plugin.
  3. Install pludin
  4. Enable plugin.

When Importing a plugin there is one additional step. Import plugin and browse to select the tarball. Then follow the above steps.

rgleason commented 1 year ago

I believe they were working on the version shown issue. Do not know if it was resolved.

Corsair-63 commented 1 year ago

Hi Rick, all steps done and shown in the plugin tab latest installed in this case 2.4.74, but if I select master catalogue and click on update the version shown exchanges to 2.4.70. if I exchange to beta catalogue, the version displayed in the plugin tab is 2.4.74 and if click on update catalogue remains such version 74.

rgleason commented 1 year ago

Thank you Corsair, Yes, I agree. Will report to OpenCPN Issues.

  1. Options > PIM > Settings > Select Active Catalog: Beta
  2. Select "Update Plugin Catalog Beta".
  3. Install Watchdog 2.4.74 and the correct version shows "2.4.74" and is installed and works.
  4. PIM>Settings > Select Active Catalog: Master
  5. Watchdog 2.4.74 is still installed and has the right version number "2.4.74"
  6. Watchdog 2.4.74 preferences shows properties Major 2 Minor 4 Patch74
  7. Options > PIM > Select "Update Plugin Catalog: Master"
  8. Now Watchdog 2.4.70 shows, the version number is wrong, but Preferences shows that version 2.4.74 is installed.
  9. Also when we go to select "Downgrade" only the 2.4.70 installation is available, so the program actually knows that it is a "Downgrade" but is not showing the correct version. There needs to be some additional checking to determine what is actually installed rather than what is available, I think.

See two screenshots.

Before-Update-Master-Catalog Update-Catalog Downgrade

leamas commented 1 year ago

Nothing strange here, the bug is in the catalog which has something like

  <plugin version="1">
    <name> Watchdog </name>
    <version> 2.4.70.0 </version>
    <release> 0 </release>
    <summary> Implement watchdog ability for opencpn </summary>
   ...
    <target>msvc-wx32</target>
    <target-version>10.0.20348</target-version>
    <target-arch>x86_64</target-arch>
   ...
  </plugin>

That is, the version as defined in the catalog is correctly reflected in the PIM plugin data, but the catalog is wrong.

rgleason commented 1 year ago

Alec are you saying that the version number shown in PIM next to each plugin is the version available in the "master" catalog and is not the version actually installed?

I find this somewhat counter intuitive. For example I think I am using version 2.4.70 when actually I am using 2.4.74. The "Downgrade" button actually gives the user a clue that there is a earlier version available, and so I think the version number shown should be the one that is actually being used "2.4.74" as shown under "Preferences"

I think this would require some checking for which plugin version is actually installed rather than looking at the available catalog versions.

rgleason commented 1 year ago

Alec, I don't believe you have fully understood the sequence here, please go back and read, The plugin was loaded from the Beta Catalog. Then the catalog was changed to Master. At that point, the version number still shows 2.4.74 and is correct.

However when the catalog is updated using master, the version number changes to be 2.4.70 which is as you say what is in the master catalog. However version 2.4.74 is still installed and you have a "Downgrade" button which acknowledges that you can install an earlier version.

leamas commented 1 year ago

This is still basically NOTABUG, or at least WONTFIX. The situation is basically that for plugins using API <= 1.17 the version displayed is the version in the catalog. IIRC there are some fixes to this in current master, but this is still the basic fact (imported plugins is the exception).

That is, changing the catalog for an installed plugin is basically to pull the rug.

The correct solution is to update the plugin to use API 1.18+. So updated, the plugin provides the complete version info using the API and the version in the catalog is not used. Until then I think we could just live with this, using the beta catalog is sort of a corner case after all.

rgleason commented 1 year ago

Alec, I completely understand this now.
API less than 1.18 do not contain complete version information (the patch part). (I had forgotten this.) It is certainly not worth changing/struggling any further, but just get the plugins all using API 1.18 properly. This will require adding some code to each plugin for the patch won't it?

I think we can close this, but I'd like to know how important it is to update to API 1.18 and when we should do this? A part of that is understanding what will change and what will have to be dealt with if anything. If it is just a change to use API 1.18, I could very simply do that when building all the TP plugins.

Once I have some information about that, I can close this.

leamas commented 1 year ago

This will require adding some code to each plugin for the patch won't it?

Yes. It's documented in shipdriver_pi UPDATE_PLUGIN.md

EDIT: Of course, this is in the shipdriver context, you will have to adapt it for TP.

EDIT2 Fixing the totally out of scope leading quote.

rgleason commented 1 year ago

Thanks Alec, leaving open.

rgleason commented 1 year ago

Of course once this is done, OpenCPN 5.6.2 plugins cannot be built unless we have added code to make the choice of API at the time of compiling, as Stevead has suggested. (I will find it).

bdbcat commented 1 year ago

Using opencpn-libs as a submodule provides the machinery to select API level at build time. eg:

    add_subdirectory(${PROJECT_SOURCE_DIR}/opencpn-libs/api-16)
    target_link_libraries(${PACKAGE_NAME} ocpn::api)

What is missing is the switch logic in the CI build scripts.

leamas commented 1 year ago

Of course once this is done, OpenCPN 5.6.2 plugins cannot be built unless we have added code to make the choice of API at the time of compiling, as Stevead has suggested. (I will find it).

Actually, you can still use API 1.17 for 5.6.2, and that includes the improved version mechanisms. Walking this path means minimizing the differences between plugins for 5.6.2 and those for 5.8.x

rgleason commented 11 months ago

Watchdog has not been updated to use SignalK or Nmea2000, so updating to api118 is not necessary, and furthermore allows WD to be compatible with O5.6.2 as well. When OpenCPN 5.10 comes out we will need to update to api 118.

rgleason commented 10 months ago

Can we close this until OPenCPN 5.10 comes out?

bdbcat commented 10 months ago

re: "When OpenCPN 5.10 comes out we will need to update to api 118." No, this should not be necessary. Might be desirable, but not required. All plugins that work with o584 will work with O5.10.

Meanwhile, on this issue: Yes, close. Reopen or crate new issue when 5.9 Beta testing starts, if necessary.

rgleason commented 10 months ago

Thanks for the clarification.