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

Update build environment and dependencies #2

Merged
merged 2 commits into from
Aug 20, 2024

Conversation

NickCao
Copy link
Contributor

@NickCao NickCao commented Jun 6, 2024

Only tested power control and storage switching.

@mangelajo
Copy link
Member

Nice work 🚀

Comment on lines 270 to 272
ConfigKey::Json => {
xfer.accept_with(&cfg.json).ok();
}
Copy link
Member

Choose a reason for hiding this comment

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

Just a note: no action is required.

We will have an issue with .json once it gets bigger than the transfer size. I don't know what the default is or what we configured.

On the software I was starting to move name/tags/usbconsole under json , as those are details that the microcontroller itself does not care about, or use for anything,.... but the transfer size could be a good reason to stop doing such thing.

Comment on lines 298 to 310
ReadKey::Power => {
let mut buf = heapless::Vec::<u8, 128>::new();
write!(buf, "{:.2}W", self.data.power).ok();
xfer.accept_with(&buf).ok();
}
ReadKey::Voltage => {
let mut buf = heapless::Vec::<u8, 128>::new();
write!(buf, "{:.2}V", self.data.voltage).ok();
xfer.accept_with(&buf).ok();
}
ReadKey::Current => {
let mut buf = heapless::Vec::<u8, 128>::new();
write!(buf, "{:.2}A", self.data.current).ok();
xfer.accept_with(&buf).ok();
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we add another command to read them all at once?
I would expect that we periodically poll during the execution of a test, and ideally we would want the three values (actually just current and voltage would give us all we need), May be ReadKey:CurrentVoltage?

}
Ok(ControlRequest::Power) => {
if let Ok(action) = req.value.try_into() {
self.power = Some(action);
Copy link
Member

Choose a reason for hiding this comment

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

ah nice, so you decouple the reception of the control commands/data, with the actuator. I guess it isolates the objects each task has access to.

let led_rx = cx.shared.led_rx;
let to_host_serial = cx.local.to_host_serial;
let led_rx = cx.shared.led_rx;
let serial2 = cx.shared.serial2;
Copy link
Member

Choose a reason for hiding this comment

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

may be now we should just call it dut_usb_serial or dut_vcp? there is no serial1

}
}
}
}

fn escaped_char(c:u8) -> Option<u8> {
Copy link
Member

Choose a reason for hiding this comment

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

so much complexity went away :D ... ❤️

@NickCao
Copy link
Contributor Author

NickCao commented Jun 11, 2024

Extracted the control interface to its own PR: #3

@mangelajo
Copy link
Member

Ooppss sorry, now this one is in merge conflict, I must have merged this long ago

@NickCao
Copy link
Contributor Author

NickCao commented Aug 20, 2024

Rebased, lemme test.

.strings(&[
StringDescriptors::new(LangID::EN)
.manufacturer("Red Hat Inc.")
.product("Jumpstarter DFU Mode")
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
.product("Jumpstarter DFU Mode")
.product("Dutlink DFU Mode")

not related to this change, but since we are at it...

Copy link
Member

@mangelajo mangelajo left a comment

Choose a reason for hiding this comment

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

I have suggestion, but let's do it later :)

@mangelajo mangelajo merged commit 0294121 into jumpstarter-dev:main Aug 20, 2024
2 checks passed
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.

2 participants