Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Project Nodes unavailable on a Device assigned to an Application #3018

Closed
knolleary opened this issue Oct 27, 2023 · 6 comments · Fixed by FlowFuse/device-agent#205, #3074 or FlowFuse/nr-project-nodes#55
Assignees
Labels
feature-request New feature or request that needs to be turned into Epic/Story details priority:high High Priority size:L - 5 Sizing estimation point
Milestone

Comments

@knolleary
Copy link
Member

Current Behavior

When a Device is assigned to an Application, not an instance, the Project nodes are unavailable.

This is because the Project Nodes expect to be operating within the context of an instance in order to establish the proper ACL checks.

This is not a regression - this has been a limitation since the ability to assigned devices to Applications was introduced. However it needs to be addressed as users expect to use the Project Nodes regardless.

Expected Behavior

The Project Nodes should be available to Application-assigned devices

Steps To Reproduce

  • Assign device to Application
  • Put in developer mode
  • open editor - note lack of Project nodes

Environment

  • FlowForge version:
  • Node.js version:
  • npm version:
  • Platform/OS:
  • Browser:

Have you provided an initial effort estimate for this issue?

I have provided an initial effort estimate

@knolleary knolleary added the needs-triage Needs looking at to decide what to do label Oct 27, 2023
@MarianRaphael MarianRaphael added feature-request New feature or request that needs to be turned into Epic/Story details priority:high High Priority labels Oct 27, 2023
@MarianRaphael MarianRaphael added this to the 1.14.4 milestone Oct 27, 2023
@MarianRaphael MarianRaphael modified the milestones: 1.14.4, 1.14 Oct 27, 2023
@knolleary knolleary added the size:L - 5 Sizing estimation point label Nov 6, 2023
@Steve-Mcl Steve-Mcl moved this from Todo to In Design in 🛠 Development Nov 8, 2023
@Steve-Mcl
Copy link
Contributor

@knolleary

What I have working thus far:

  1. App Assigned Device can broadcast (and other instances / devices) can receive them
  2. App Assigned Device can send direct to an instance (that instance can of course be an instance assigned device)
  3. Can perform link-call to an instance
app-device-proj-link.mp4

Missing functionality:

  1. An instance or other device CANNOT send direct to an App Assigned Device
  2. An instance or other device CANNOT link-call to an App Assigned Device

Having had a bit of a chew on getting to this point (dev env issues, debugging issues, etc) I must apologise for forgetting the exact details of our chat last Monday. That leads to the question:

Is it enough in this iteration to support the "working set" (as listed in the first 3 points) - OR - is it desirable to get the "missing functionality" implemented too?


CC @MarianRaphael ^

@knolleary
Copy link
Member Author

The goal is a minimal set of functionality that starts filling the gap caused by the absence of the project nodes in an App-assigned device.

Lets get what you've done so far into a PR so we can start reviewing. We need to make sure the MQTT topic structures are appropriate and your video doesn't show anything about how these new options are exposed in the UI.

We will want to be able to messaging directed at the Device, but that can be a second iteration.

@Steve-Mcl
Copy link
Contributor

We need to make sure the MQTT topic structures are appropriate

There are no changes to the topic structure. That may be necessary to support direct device messaging. And as I move onto supporting direct device messaging I may even discover we have locked ourselves into a less than extensible format. However there is always the V1 topic section to fix that if necessary.

your video doesn't show anything about how these new options are exposed in the UI

There are no changes to the UI for the current level of support I listed in the first 3 points. However as allude to next, I believe that may be necessary (or rather highly desirable)

We will want to be able to messaging directed at the Device, but that can be a second iteration

Understand.

There is a fair bit of design discussion required around direct to device messaging that needs to be understood. For example, we currently show the user a dropdown list of instances. If we simply add devices to this list, larger users will find the UX pretty uncomfortable. Next there is the scoping - e.g. would we even want to list devices in different applications along side devices in this application. Finally there is making the project nodes application aware.

I think a short brainstorming session would really help with the remaining pieces.

@Steve-Mcl
Copy link
Contributor

your video doesn't show anything about how these new options are exposed in the UI

There are no changes to the UI for the current level of support

Correction. There are minimal wording changes to the UI and help to indicate devices are now ☺️

@knolleary
Copy link
Member Author

Reopening as there are multiple PRs needed to land for this to be complete.

@Steve-Mcl Steve-Mcl removed the needs-triage Needs looking at to decide what to do label Nov 21, 2023
@Steve-Mcl
Copy link
Contributor

Following the Verification procedure written up in this PR, I have confirmed it works as expected.

instance-to-app-device-2023-11-22--21-50.mp4

Points of note:

  1. A device sat in dev mode will NOT be instructed to update (switching out of dev mode will cause the difference in modules to be detected and thus update
  2. On an app owned device, the Project In node might not be as clear as it could be.
    image
    Additional context: App assigned devices can not be chosen as a target in the Project-out node at in this 1st iteration.
    This option being enabled may cause a user to go looking for "how to send direct message#" (or report an issue)
    We might want to consider disabling this option in an app owned device
  3. An already created instance needed to be restarted before it would respond to a link-call (was getting "Project Link Source not valid") which suggested there was something wrong with the messageEvent object. I thoroughly checked the data in the msg and all appeared correct. I came to the conclusion that the instance needed a restart in order to load latest project nodes - doh. But it tripped me up for some time - not certain how we can handle this kind of situation?

RE Point 2 - I will raise an issue and PR before launch. (discussed here: https://flowforgeworkspace.slack.com/archives/C032Q63FGG1/p1700682292559729)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature or request that needs to be turned into Epic/Story details priority:high High Priority size:L - 5 Sizing estimation point
Projects
Archived in project
Archived in project
3 participants