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

[WIP - To discuss, DO NOT merge] Add STM boards #77

Open
wants to merge 40 commits into
base: master
Choose a base branch
from

Conversation

LoicGRENON
Copy link
Contributor

Here is a work in progress to add the STM boards supported by TeamGloomy to the ConfigTool.
I first wanted to separate these boards from the Duet ones by adding them into store/STMBoards.ts instead of store/Boards.ts
but I couldn't find an easy way to expand the Boards Record descriptor without having to change a lot of files ...
What are your thoughts about this ?

jaysuk and others added 30 commits July 7, 2023 16:48
…'t get shrink because of the indent function
- Display STM board only when a checkbox is checked
…hortnames was too vague

- Change the caption label of the checkbox used to enable STM boards
e.g : Super8Pro cannot be used for both STM32H723BoardType and STM32H743BoardType
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.vscode files are ignored by .gitignore. Please remove this file from git.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this? I see no benefit.

Copy link
Contributor Author

@LoicGRENON LoicGRENON Mar 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is to hide the STM boards when the disclaimer checkbox is unchecked. This is related to that section.

@@ -43,10 +44,14 @@ export interface BoardDescriptor extends BaseBoardDescriptor {
supportsBME280: boolean;
}

export type BoardTypes = BoardType | STMBoardTypes
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really necessary to export both?

Copy link
Contributor Author

@LoicGRENON LoicGRENON Mar 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For STMBoardTypes, I added a property stm to store the info needed and related to each board.
The easiest way I found was to add these boards to the Boards descriptor and by defining BoardTypes as this.

I would have prefered to have an independant descriptor record for the STMboards (for example defined in store/STMBoards.ts as it would be easier to maintain) but I didn't found an easy way to do this (and without needing to modify many files ...).
That's actually why I created this PR : to discuss about this solution.

@@ -230,6 +230,7 @@ defaultTemplate.configTool.assignPort("out0", ConfigPortFunction.heater, 0); //
defaultTemplate.configTool.assignPort("out1", ConfigPortFunction.heater, 1); // Nozzle heater
defaultTemplate.configTool.assignPort("out3", ConfigPortFunction.fan, 0); // Nozzle fan
defaultTemplate.configTool.assignPort("out4", ConfigPortFunction.fan, 1); // Nozzle thermostatic fan
defaultTemplate.configTool.enableSTMBoards = true;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The official ConfigTool cannot enable STM boards by default unless a disclaimer is confirmed stating that your boards are third-party boards and not developed or maintained by Duet3D. This disclaimer should show up when you click on the checkbox to enable STM boards.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I know but this PR is to discuss about the way I expanded the Boards Record descriptor.
I just forgot to remove this before the PR and I defined this default value to not have to check the checkbox each time I want to test something.
This will be indeed removed on the final PR.
This PR is not intended to be merged.

@LoicGRENON LoicGRENON changed the title [WIP] Add STM boards [WIP - To discuss, DO NOT merge] Add STM boards Mar 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants