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

USB Protection Improvement and ExamMode Fix #111

Closed

Conversation

devdl11
Copy link
Member

@devdl11 devdl11 commented Dec 27, 2021

Some little improvements preparing the release

@devdl11 devdl11 marked this pull request as draft January 9, 2022 17:01
@devdl11 devdl11 marked this pull request as ready for review January 29, 2022 20:20
@github-actions
Copy link

.text .rodata .bss .data Total (RAM) Total (ROM)
Base 809936 bytes 434244 bytes 226880 bytes 2304 bytes 229184 bytes 1246484 bytes
Head 811592 bytes 433884 bytes 227024 bytes 2304 bytes 229328 bytes 1247780 bytes
+1656 bytes -360 bytes +144 bytes +0 bytes +144 bytes +1296 bytes
+0.2 % -0.1 % +0.1 % +0.0 % +0.1 % +0.1 %

Comment on lines 116 to 118
#if ION_SIMULATOR_FILES
mode = GlobalPreferences::ExamMode::Off;
#else
Copy link
Member

Choose a reason for hiding this comment

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

I think these lines should be removed, because this pull request is marked as ready for review

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know because it add the possibility to disable the exam mode in the simulator without rebooting

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but it is marked as ready to merge…

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed because I think it can be a good idea to add the possibility to disable the exam mode in simulators ^^ The exam mode in simulator doesn't make any sense in the first place because you can always leave the simulator and open others app so I don't see the problem by adding the option

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but if we look #131, it is built as a simulator, but it runs on the calculator…

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I agree with Yaya-Cout

@@ -78,6 +78,7 @@ class Function : public ExpressionModelHandle {
}
bool isActive() const { return m_active; }
void setActive(bool active) { m_active = active; }
void setMyColor(KDColor color) {m_color = color; }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
void setMyColor(KDColor color) {m_color = color; }
void setMyColor(KDColor color) { m_color = color; }

Copy link
Member

Choose a reason for hiding this comment

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

In fact, is it possible to give a better name to the function?

Copy link
Member Author

Choose a reason for hiding this comment

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

setGraphColor ?

@@ -245,7 +245,7 @@ void __attribute__((noinline)) start() {
Ion::Device::Board::initFPU();

/* Call static C++ object constructors
* The C++ compiler creates an initialization function for each static object.
* The C++ compiler creates an initialization function for each static object.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* The C++ compiler creates an initialization function for each static object.
* The C++ compiler creates an initialization function for each static object.

@@ -78,6 +78,7 @@ void DFUInterface::wholeDataSentCallback(SetupPacket *request, uint8_t *transfer
// Leave DFU routine: Leave DFU, reset device, jump to application code
leaveDFUAndReset();
} else if (m_state == State::dfuDNBUSY) {
m_state = State::dfuDNBUSY;
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand this line...

@@ -188,7 +189,7 @@ void DFUInterface::changeAddressPointerIfNeeded() {

void DFUInterface::eraseCommand(uint8_t *transferBuffer, uint16_t transferBufferLength) {
/* We determine whether the commands asks for a mass erase or which sector to
* erase. The erase must be done after the next getStatus request. */
* erase. The erase must be done after the next getStatus request. */
Copy link
Member

Choose a reason for hiding this comment

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

?

Copy link
Member

@Yaya-Cout Yaya-Cout Feb 3, 2022

Choose a reason for hiding this comment

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

I think it has been squashed when merging, but this fork isn't up-to-date, so it isn't corrected in this repo

@@ -152,7 +153,7 @@ bool DFUInterface::processUploadRequest(SetupPacket *request, uint8_t *transferB
} else {
/* We decided to never protect Read operation. Else we would have to check
* here it is not protected before reading. */

Copy link
Member

Choose a reason for hiding this comment

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

?

@github-actions
Copy link

github-actions bot commented Feb 4, 2022

.text .rodata .bss .data Total (RAM) Total (ROM)
Base 809936 bytes 434244 bytes 226880 bytes 2304 bytes 229184 bytes 1246484 bytes
Head 811592 bytes 433884 bytes 227024 bytes 2304 bytes 229328 bytes 1247780 bytes
+1656 bytes -360 bytes +144 bytes +0 bytes +144 bytes +1296 bytes
+0.2 % -0.1 % +0.1 % +0.0 % +0.1 % +0.1 %

Copy link
Member

@Yaya-Cout Yaya-Cout left a comment

Choose a reason for hiding this comment

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

Can you fetch upstream, and test with the trash ?

Comment on lines +116 to +120
// #if ION_SIMULATOR_FILES
// mode = GlobalPreferences::ExamMode::Off;
// #else
mode = GlobalPreferences::sharedGlobalPreferences()->examMode();
// #endif
Copy link
Member

Choose a reason for hiding this comment

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

I think these comments can be removed.

@@ -217,7 +216,7 @@ void DFUInterface::eraseMemoryIfNeeded() {
if (m_erasePage < 0) {
return;
}

Copy link
Member

Choose a reason for hiding this comment

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

Remove trailing space…

Suggested change

@Yaya-Cout
Copy link
Member

I think this pull request have to be updated, because #205 remove the USB protection…

@devdl11 devdl11 closed this Nov 4, 2023
@devdl11
Copy link
Member Author

devdl11 commented Nov 4, 2023

I close this pull request because it's too outdated to be merged, a completed rework needs to be made

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