PIM beta catalogue display discussion #3473

rgleason · 2023-10-22T10:58:27Z

Describe the bug
PIM beta catalog display is inconsistent. This needs to be improved and may need some discussion about how to handle it.
https://www.cruisersforum.com/forums/f134/pim-beta-catalogue-display-280643.html#post3833686

To Reproduce
Steps to reproduce the behavior:

PIM Beta Catalog
See how Internal and External Plugins are displayed
See how master catalog plugins are handled.
Try updating each type.
What should be displayed?
What plugins should be allowed to work?
What plugins should even be shown?

Expected behavior

This needs more discussion

Screenshots
https://www.cruisersforum.com/forums/f134/pim-beta-catalogue-display-280643.html#post3833686

Desktop (please complete the following information if applicable):
All OS

Perhaps this should be converted to a discussion.

Corsair-63 · 2023-10-22T12:23:04Z

PIM Beta Catalog

See how Internal and External Plugins are displayed

See how master catalog plugins are handled.

Try updating each type.

What should be displayed?

What plugins should be allowed to work?

What plugins should even be shown?

Switched to Beta
Internal plugins are displayed as available update when the available version is the same as installed and they are not displayed as the gear wheel. external plugins are correctly displayed (FTB).
Exchanged to master catalogue, no display problem of plugins (at this time I have no plugin to update. and the internal plugins are displayed properly. except those that are installed as beta and have higher version than master catalogue and are shown with master catalogue version instead.
done in both cases as per above screenshots.
in Beta, should be displayed all of them irrespective of the source the ones in Beta state eligible to update with the arrow, other with same version with the thick and the greyed dot, I don't know by now what does mean? (only suspecting). in the master catalogue for now there is a display bug that shows the latest master version but in case that installed newer beta plugin, it doesn't show this version only shows the master version even this one is lower. (already reported in separate message but cannot reproduce at this time) At this time all plugins installed are in master catalogue with latest version and no newer beta plugin installed.
??? the one installed?
That answered in 5.

this has happened in O 5.8.4-0 windows version, Windows 11 Pro x64 & ARM64.

the expected behaviour is answered in point 5, or what I think from it (IMO):
when there is an update irrespective of the catalogue used, an arrow, if inside this catalogue, show the updates for master in master show the Beta in Beta, I mean if there is an update in Beta but not in master show it in beta but not in master.
if the version installed of plugin is higher (because installed beta, alfa, etc) than available master update, show the latest version installed in the PIM and not the master version.

leamas · 2023-10-22T12:45:40Z

This about the built-in, system plugins. For some reason the core fails to identify them as such when using the Beta catalog. I'll take a look into this later.

For now, the obvious work-around is to not try to do anything besides activating/deactivating these four plugins WMM, ChartDowloader, GRIB and Dashboard. In particular, they should not be updated.

leamas · 2023-10-22T12:47:32Z

The obvious question: which version of OpenCPN is being used when these errors occur?

Corsair-63 · 2023-10-22T12:49:25Z

The obvious question: which version of OpenCPN is being used when these errors occur?

after my answer at point 7 is the version of O & o.s.

Corsair-63 · 2023-10-22T12:50:25Z

For now, the obvious work-around is to not try to do anything besides activating/deactivating these four plugins WMM, ChartDowloader, GRIB and Dashboard. In particular, they should not be updated.

absolutely agreed with your answer

leamas · 2023-10-22T13:42:04Z

While I can reproduce this on 5.8.4 the problem is seemingly not present on a build from current master at d6c4868.

This is not a big surprise, there has been some work in these areas since 5.8.4.

There is a copy of my build at https://dl.cloudsmith.io/public/alec-leamas/opencpn/raw/files/opencpn_5.9.0-0+d6c4868ff_setup.exe if anyone more could test. This would be very much appreciated.

Corsair-63 · 2023-10-22T15:00:39Z

Hi Leamas,

thanks,
started testing yours above, as far as I see it seems bit faster.
ConfigVersionString=Version 5.9.0-0+d6c4868ff Build 2023-10-22 (windows 11 (x64) Pro 23H2 ES)

of course, as you could understand I'm testing at home, not in in my boat, just for safety reasons and not making a parallel installation.

the PIM issue in beta catalogue doesn't happens.

I'll test further if happens also with beta versions higher than master version and what is shown in master catalogue versioning, for such situation I'll need some beta plugin with higher version than master and check availability in master catalogue, to see how it would be displayed?

leamas · 2023-10-22T15:04:55Z

It will be displayed the same way. The four system plugins this is all about are not really managed as other plugins, they are part of the opencpn build and are more or less as-is until next OpenCPN release.

In particular, they are not really part of the catalog and they cannot be updated.

leamas · 2023-10-22T15:09:05Z

BTW: thanks for testing!

I tend to think that this issue should be closed since we for better or worse close issues which are fixed in master. I really miss the opportunity to close with a specifed resolution like "FIXED-NEXTRELEASE" or so.

OTOH, the culture here is that @rgleason as the original reporter makes this decision.

Corsair-63 · 2023-10-22T15:12:22Z

It will be displayed the same way. The four system plugins this is all about are not really managed as other plugins, they are part of the opencpn build and are more or less as-is until next OpenCPN release.

In particular, they are not really part of the catalog and they cannot be updated.

e.g. just for example:
Any plugin, imaging this:

Beta version 0.4.0.201
Master version 0.4.0.200
installed version 0.4.0.201

when switch to beta catalogue of course won't be any update to plugin because installed latest beta version.
switched to master and check availability, now it will show version 200 (I understand like installed), when should show 201, yes, it's a beta version, but is the latest, for such reason when you click on it you use to have the option of downgrade version in such cases and install the master one 200)

rgleason · 2023-10-22T15:45:06Z

Will close with "FIXED-NEXTRELEASE" when testin/discussion is done. Thanks to you both!

leamas · 2023-10-22T17:20:25Z

This has been discussed in #3438. It's basically a question about updating the plugins so the core can get the version from the plugin interface rther than the catalog.

Which means that this is not really about core OpenCPN which is the target for any issue filed here.

leamas · 2023-10-22T18:24:28Z

@Corsair-63 ^

rgleason · 2023-10-23T00:17:11Z

Okay, so the problems should be pretty much fixed for the TP plugins, as they are all API117 now (or 118)

leamas · 2023-10-23T06:26:00Z

@rgleason : Yes, as long as you have added the functions described in point 6 in the shipdriver API 18 README

rgleason · 2023-10-23T09:59:39Z

Have intended to add version_patch and version_tweak to all tp pi in the last major push along with api 117 changes. Next version of o it might be better.

But I ask, is O working between pi catalogs the way we want it to?

I have just used the catalogs and not thought about it much.

leamas · 2023-10-23T11:15:42Z

The important thing is that you add the methods, like

int GetPlugInVersionPatch() { return ...; }
int GetPlugInVersionPost() { return ...; }
const char *GetPlugInVersionPre() { return ...; }
const char *GetPlugInVersionBuild() { return ...; }

These must be defined, otherwise the plugin most likely won't load or run into runtime errors. The could return anything, but they must exist.

rgleason · 2023-10-23T11:56:17Z

Gernerally, for TP plugins, we have:

in [plugin]_pi.cpp

int aisradar_pi::GetAPIVersionMajor() {    return OCPN_API_VERSION_MAJOR; }
int aisradar_pi::GetAPIVersionMinor() {    return OCPN_API_VERSION_MINOR; }
int aisradar_pi::GetPlugInVersionMajor() {  return PLUGIN_VERSION_MAJOR; }
int aisradar_pi::GetPlugInVersionMinor() {  return PLUGIN_VERSION_MINOR;}
int aisradar_pi::GetPlugInVersionPatch() {   return PLUGIN_VERSION_PATCH;}
int aisradar_pi::GetPlugInVersionPost() {   return PLUGIN_VERSION_TWEAK;}

in [plugin]_pi.h

    int GetAPIVersionMajor();
    int GetAPIVersionMinor();
    int GetPlugInVersionMajor();
    int GetPlugInVersionMinor();
    int GetPlugInVersionPatch();
    int GetPlugInVersionPost();
    wxBitmap *GetPlugInBitmap();
    wxString GetCommonName();
    wxString GetShortDescription();
    wxString GetLongDescription();
    wxBitmap m_panelBitmap;

I could have missed one plugin.

leamas · 2023-10-23T12:03:39Z

If compare to the list above you'll notice that there are missing functions in both plugins. You must add them, even if they just return "".

rgleason · 2023-10-23T12:14:02Z

Nice, so you'd have me rebuild all the plugins and deploy and then push to plugins because
These are missing?

int GetPlugInVersionPatch() { return ...; }
int GetPlugInVersionPost() { return ...; }
const char *GetPlugInVersionPre() { return ...; }
const char *GetPlugInVersionBuild() { return ...; }

There is no way, I don't have enough credits. Too bad I was not informed about this when I did it.
It will have to wait until the next cycle, worst case is 3 months.
Is the above correct now?

leamas · 2023-10-23T12:20:16Z

I have pointed you to the shipdriver update page multiple times, all this is there.

You obviously cannot return ..., that is a placeholder. In the shipdriver case we have suitable constants for this, but if there are no such available you could use ``""`

While you are on it: All the definitions in [plugin]_pi.h are duplicates of ocpn_plugin.h. They should be removed, it's at best confusing. ocpn_plugin.h is the only source of truth for these.

rgleason · 2023-10-23T12:22:04Z

Okay, using https://github.com/rgleason/AISradar_pi
What do you want me to change in all the plugins, just so we get it right once, this time.
Your expectations are high, as I am not really a programmer.

Re ... okay so what does it return?

Also I can't know all this stuff about plugin_pi "[cruft]" seems to be a new word thrown about...

How would I know or keep track of various changes made in shipdriver? I get all the Issue messages, I didn't see anything about this major feature that you and the rest of us would very much like to have work. Verfy glad we are having this come to light now.

When you just try to push the issue away down to plugins, because it is not related to OpenCPN, and the issue is the interrelatedness of plugins with opencpn and the versioning that is shown in OpenCPN, that does not really help get to the nub.

leamas · 2023-10-23T12:32:55Z

I'll try make a PR. MY assumptions:

There are no provisions in the TP template for a string like beta-1 in the version, besides of course if the versions is tagged.
There is no build info like git hash and/or build number available during the build (added in the upload phase?).

Correct me if I'm wrong in any of these

leamas · 2023-10-23T12:51:08Z

This is challenging, most of the plugin methods are wrong. Will take while.

rgleason · 2023-10-23T12:52:08Z

Alec, I am sorry about this. I won't be able to get to it until end of December (see my email). I defer to @bdbcat re your questions. You can take some time to do the PR. Thank you!

leamas · 2023-10-23T13:25:24Z

This has morphed to something else. It should be closed since the original issue is solved. We could continue anyway.

bdbcat · 2023-10-23T14:46:27Z

Alec...
I got this.
Q: Not sure what is expected to be returned by these functions

  /** Pre-release tag version part, see GetPlugInVersionPatch() */
  virtual const char *GetPlugInVersionPre();

  /** Build version part  see GetPlugInVersionPatch(). */
  virtual const char *GetPlugInVersionBuild();

Hints? How used?

leamas · 2023-10-23T15:03:19Z

They are more straight forward parts from the https://semver.org specification, the pre-release and build metadata paragraphs.

In shipdriver, the GetPlugInVersionPre is usually empty, but in particular radar_pi uses a workflow where versions get a part like "beta1" or beta2" which then is reflected here.

For GetPlugInVersionBuild() shipdriver generates a build identification like 123.deadbee where the first digits is the build number and the rest a git commit. I have made a quick look into this for aisradar, but TP don't have these available in the build, they are defined when uploaded. The minmum is to just return an empty string

leamas · 2023-10-23T15:06:06Z

There is a PR against AISRadar_pi which fixes the minimum

bdbcat · 2023-10-23T15:17:55Z

Actually, I think we do not need to implement this at all, if we do not want to right now.
The methods already exist in core ocpn_plugin.cpp:273

const char* opencpn_plugin_117::GetPlugInVersionPre() { return ""; };

const char* opencpn_plugin_117::GetPlugInVersionBuild() { return ""; };

No TP plugin has trouble loading now due to missing these functions. So the core default implementations appear to be being used.

So, what will be functionally broken in OCPN PIM management if these functions return ""?

leamas · 2023-10-23T15:25:51Z

nothing, my bad. I did everything right when I wrote it, now it's all forgotten (:

rgleason · 2023-10-23T15:35:35Z

Thanks Alec, very much appreciated!. Also Dave.
Sorry just merged it. We can revert if needed.

rgleason · 2023-10-23T15:36:28Z

Can I close now?

rgleason · 2023-10-23T15:38:03Z

No i cant OP has to close

leamas · 2023-10-23T15:55:06Z

Or I. Closing.

@Corsair-63 Sorry for hijacking your issue for unrelated discussions!

Corsair-63 · 2023-10-23T15:56:08Z

never mind, thanks a lot

rgleason added the bug label Oct 22, 2023

rgleason changed the title ~~PIM beta catalogue display corrections~~ PIM beta catalogue display discussion Oct 22, 2023

leamas self-assigned this Oct 22, 2023

rgleason mentioned this issue Oct 23, 2023

PIM manager Correct the variables and API calls jongough/testplugin_pi#354

Closed

leamas closed this as completed Oct 23, 2023

PIM beta catalogue display discussion #3473

PIM beta catalogue display discussion #3473

Comments

rgleason commented Oct 22, 2023 • edited

Corsair-63 commented Oct 22, 2023

leamas commented Oct 22, 2023

leamas commented Oct 22, 2023

Corsair-63 commented Oct 22, 2023

Corsair-63 commented Oct 22, 2023

leamas commented Oct 22, 2023

Corsair-63 commented Oct 22, 2023

leamas commented Oct 22, 2023

leamas commented Oct 22, 2023 • edited

Corsair-63 commented Oct 22, 2023

rgleason commented Oct 22, 2023

leamas commented Oct 22, 2023

leamas commented Oct 22, 2023

rgleason commented Oct 23, 2023 • edited

leamas commented Oct 23, 2023

rgleason commented Oct 23, 2023

leamas commented Oct 23, 2023

rgleason commented Oct 23, 2023 • edited

leamas commented Oct 23, 2023 • edited

rgleason commented Oct 23, 2023

leamas commented Oct 23, 2023 • edited

rgleason commented Oct 23, 2023 • edited

leamas commented Oct 23, 2023

leamas commented Oct 23, 2023

rgleason commented Oct 23, 2023

leamas commented Oct 23, 2023

bdbcat commented Oct 23, 2023 • edited

leamas commented Oct 23, 2023

leamas commented Oct 23, 2023

bdbcat commented Oct 23, 2023

leamas commented Oct 23, 2023

rgleason commented Oct 23, 2023

rgleason commented Oct 23, 2023

rgleason commented Oct 23, 2023

leamas commented Oct 23, 2023

Corsair-63 commented Oct 23, 2023

rgleason commented Oct 22, 2023 •

edited

leamas commented Oct 22, 2023 •

edited

rgleason commented Oct 23, 2023 •

edited

rgleason commented Oct 23, 2023 •

edited

leamas commented Oct 23, 2023 •

edited

leamas commented Oct 23, 2023 •

edited

rgleason commented Oct 23, 2023 •

edited

bdbcat commented Oct 23, 2023 •

edited