search

Beckhoff-USA-Community / SPT-Libraries

MIT License
88 stars 21 forks source link

Whitelisting HMI variables #24

Open kaelev7 opened 1 year ago

kaelev7 commented 1 year ago

Is there any reason for not using whitelisting or blacklisting for the HMI structs?

For me, this sounds quite useful, especially the whitelisting with { attribute 'TcHmiSymbol.ShowRecursively' } or { attribute 'TcHmiSymbol.Show' } to give the HMI folks only the variables they need. And prevent unexpected things from happening if they use the wrong variables.

nshiggins commented 1 year ago

This is a good idea I will bring it up with the team.

On Wed, Aug 9, 2023 at 09:34 Jörg Maier @.***> wrote:

Is there any reason for not using whitelisting or blacklisting for the HMI structs?

For me, this sounds quite useful, especially the whitelisting with { attribute 'TcHmiSymbol.ShowRecursively' } or { attribute 'TcHmiSymbol.Show' } to give the HMI folks only the variables they need. And prevent unexpected things from happening if they use the wrong variables.

— Reply to this email directly, view it on GitHub https://github.com/Beckhoff-USA-Community/SPT-Libraries/issues/24, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEMHY7NH3NE5UIK6FS6UFILXUOGVDANCNFSM6AAAAAA3KATBLQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

albertgoodwin commented 1 year ago

I'm interested in other people's experience with this. For me, it seems simpler to leave whitelisting false and use the { attribute 'TcHmiSymbol.Hide' } to deliberately hide variables i don't want available to an HMI in my FBs or (in a library). With whitelisting i find i have to add a lot more attributes on many levels.

kaelev7 commented 1 year ago

@albertgoodwin I agree with getting other options to find a good solution. And not having to much attributes in the source.

But if I get you right, you will give nearly all variables to the HMI? So we need to trust that no one writes into the wrong variable from the HMI, otherwise it's hard to find the bug.

I would only whitelist the HMI structs, and everything else is not visible to the HMI. If other variables are needed, they must be inside these structs. So there is no need to whitelist a lot of variables.

nshiggins commented 1 year ago

This is still very much on the roadmap and I'm investigating the best way to implement across the libraries. I did some experimentation already and the results were somewhat mixed. I was pulled in a different direction and haven't been able to get back since.

I prefer whitelisting vs. hiding as the design intent was to provide all the necessary HMI hooks in the _HMI structures. Basically, if you are having to map symbols outside of those structures then that's something that should probably be added to the structures to begin with.

If it does make it into a release, it is almost certainly going to have to be explicitly enabled through a compiler directive--trying very hard not to break userspace :-)