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

Add groups to programming for ease of reading #1447

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MrD-RC
Copy link
Collaborator

@MrD-RC MrD-RC commented Feb 8, 2022

This PR allows people to group sections of the Programming. This is to help make the code easier to read, without having to have empty lines of code as separators.

It requires firmware update iNavFlight/inav#7812

A video of this in action. Since recording the video, I have made some subtle cosmetic tweaks. Mostly how the divider lines appear.

@error414
Copy link
Contributor

error414 commented Oct 17, 2024

I tested it and it works nicely.
https://github.com/error414/inav-configurator/tree/MrD-Add-groups-to-programming-for-ease-of-reading

There is one issue with renderCodeGroups function, function is not visible in logicCondition.js due to new namespaces in electron, I made renderCodeGroups global by

window.renderCodeGroups = renderCodeGroups;

It's not nice solution, it has to be solve in beter way

@MrD-RC
Copy link
Collaborator Author

MrD-RC commented Oct 17, 2024

We decided not to use the PR as the GUI display options shouldn't be stored on the FC. It should be stored on the PC/Configurator. Ideally in the cloud somewhere, so that the groups will work on and computer for that flight controller.

@error414
Copy link
Contributor

ahh, it's a pity :(

@sensei-hacker
Copy link
Collaborator

sensei-hacker commented Oct 18, 2024

We decided not to use the PR as the GUI display options shouldn't be stored on the FC. It should be stored on the PC/Configurator.

As I recall, the concern was that each line gets a new byte, the section number, correct?

It occurs to me a slight change to the implementation would make it there would be NO storage cost at all, when the feature isn't actively being used. That could be done in the same spirit as the html BR or HR tags or the rtf "page break" and "paragraph break" tags. We could add a PLC command for "section break".

In that way, any existing PLC would be stored exactly the same way it has always been stored. Only if the user chose to mark a section break would any bytes be used for that.

IMHO if you have a chunk of code for setting the VTX power and a separate chunk of code for adjusting the throttle, those are separate LOGICALLY. Demarcating separate units is a semantic thing, IMHO (and in the view of those who created the HTML standard). The html standard considers DIV or HEADER to be a semantic fact because the different sections are logically separate, while "blue" is a display issue controlled by CSS.

The display decision is whether to display a horizontal bar between units, highlight them in different colors, draw a box around each unit ...

The fact that there are different units or sections doesn't imply anything about how they should be displayed, it seems. And we could allow section breaks without incurring any cost at all for those who don't choose to use them. (Or when you don't choose to use them.)

@MrD-RC
Copy link
Collaborator Author

MrD-RC commented Oct 18, 2024

No, it's a decision based on not having code or parameters in the firmware which solely deal with UI. That would be the only reason they exist. So the firmware is not the right place to keep them.

Currently, each logic line would have an extra byte. The byte itself isn't the issue. It's that the pure UI data is in the firmware at all. So having separate parameters for the grouping would result in less data. But the problem is the data is in the firmware at all.

@error414
Copy link
Contributor

error414 commented Oct 18, 2024

As I see it, saving groups to PC or to cloud has as many disadvantages that may be would better not to implement groups. Local storage is good as cache, not for data which are important for users.

I would bet that all users who have more than one script in programming framework use logic condition line as separator beetween scrips. If you use coloured groups you don't have to use logic condition line as separator, but if you lost somehow saved groups info, then it will be mess in programming framework.

I understand why you don't want to have GUI params saved in FC, but it's only single solution how to do it user friendly.

@MrD-RC
Copy link
Collaborator Author

MrD-RC commented Oct 18, 2024

I can see your point. Maybe an implementation like @sensei-hacker suggested with a separate logic_display parameter would be better. Then it's only used if logic is used. Though after 32 LCs being used, it would take up more memory.

But yes, I use an LC as a space between sections of logic.

@error414
Copy link
Contributor

I like it, it's a simple solution,

"render empty conditions with a slightly darker background. That would make them visually delimit the sections nicely, perhaps."

@MrD-RC
Copy link
Collaborator Author

MrD-RC commented Oct 18, 2024

But that would still need storing somewhere. Or do you just mean the empty LC is displayed differently? In which case, we still lose an LC wherever we want a space.

@sensei-hacker
Copy link
Collaborator

sensei-hacker commented Oct 18, 2024

But yes, I use an LC as a space between sections of logic.

I do too. Since you and I create a lot of the examples others learn from, it's a good bet that some others do too.
This image has no changes at all to the firmware, just CSS in Configurator:

image

The styling in the image above is for example only. We could instead do a color shift or any other type of display we choose.

Or do you just mean the empty LC is displayed differently? In which case, we still lose an LC wherever we want a space.

Yes, there's cost - always. But we only pay that cost when we use a break, so there is no cost to anyone who doesn't use the feature. We have 64 LCs now, so we can spare two for breaks when they are needed.

The styling for the breaks as shown in the image is:

.mixer-table .off {
    opacity: 0.50
}

.mixer-table .off td,
.mixer-table .off th {
    padding-bottom: 1em;
    padding-top: 1em;
    background-color: #b8c0c0
}


.mixer-table tr:nth-child(even).off td,
.mixer-table tr:nth-child(even).off th {
    background-color: #b8c0c0;
}

That could absolutely be changed. Aesthetics isn't my thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants