Skip to content

Ents sdi12 transmit#25

Open
stevegtaylor wants to merge 54 commits intoents-devfrom
ents-sdi12-transmit
Open

Ents sdi12 transmit#25
stevegtaylor wants to merge 54 commits intoents-devfrom
ents-sdi12-transmit

Conversation

@stevegtaylor
Copy link
Copy Markdown
Member

Pull Request Overview

This pull request adds/changes/fixes...

Testing Strategy

This pull request was tested by...

TODO or Help Wanted

This pull request still needs...

Documentation Updated

  • Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make prepush.

stevegtaylor and others added 30 commits September 3, 2025 13:54
Bumps [cryptography](https://github.com/pyca/cryptography) from 43.0.1 to 44.0.1.
- [Changelog](https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst)
- [Commits](pyca/cryptography@43.0.1...44.0.1)

---
updated-dependencies:
- dependency-name: cryptography
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
The streaming process slice would write data leaving a one-byte gap
between the header and payload. This caused a mismatch between the
kernel behavior and the documented interface or assumptions made in
userspace respectively.
The board isn't used and the company behind it seems to no longer exist.
MachineRegister is a data type that represents exactly what can be
stored in a hardware-specific register. It has no additional semantic
information.
These data types in Tock have no semantic meaning other than they are
exactly the size of a register. To express that in code we use the
MachineRegister type.
These used to be CapabilityPtr and thus now need to be handled slightly
differently.
Previously, `CapabilityPtr` wrapped `MachineRegister`. This initially made sense because every `CapabilityPtr` is logically a `MachineRegister` but not every `MachineRegister` is a `CapabilityPtr`. However, we've realized that building `CapabilityPtr` atop `MachineRegister` requires `MachineRegister` to expose all of `CapabilityPtr`'s functionality, which is not correct for `MachineRegister`. Therefore, this commit reverses that relationship: `MachineRegister`'s implementation now wraps `CapabilityPtr`.

There is no conceptual change in what the two types mean. However, `CapabilityPtr` now promises that it can contain an integer for `MachineRegister`'s use -- but other code is not allowed to rely on that.
`From` is only meant for lossless conversions. Converting a `CapabilityPtr` or `MachineRegister` to a `usize`, or a `MachineRegister` to a `CapabilityPtr`, conceptually lose information. Therefore, these conversions should be inherent methods instead.
This documents how `CapabilityPtr` and `MachineRegister` interact with pointer provenance.
…en they are offset, and remove confusing redundant wording.
@stevegtaylor
Copy link
Copy Markdown
Member Author

@tyler-potyondy I fixed merge conflicts and went through the CI checks. I think it's ready for you to take a look at.

Copy link
Copy Markdown
Collaborator

@tyler-potyondy tyler-potyondy left a comment

Choose a reason for hiding this comment

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

Just did a first pass through the files. Let me know if you have any questions on the comments.

Make sure you go through and remove the debug! statements throughout these changes. These add a lot of code size.

Comment on lines +24 to +32
.PHONY: flash-kernel-app-blink
flash-kernel-app-blink: $(TOCK_ROOT_DIRECTORY)target/$(TARGET)/release/$(PLATFORM).bin erase-all
# Flash the kernel.
$(OPENOCD) -f interface/stlink.cfg -c "transport select hla_swd" -f target/stm32wlx.cfg \
-c "init; reset halt; program $< verify 0x08000000; reset; shutdown"
# Flash the blink app we temporarily are including in seeed studio board directory.
$(OPENOCD) -f interface/stlink.cfg -c "transport select hla_swd" -f target/stm32wlx.cfg -c \
"init; reset halt; program $(TOCK_ROOT_DIRECTORY)boards/seeed_studio_lora-E5-HF/blink.tbf verify 0x08020000; reset; shutdown"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
.PHONY: flash-kernel-app-blink
flash-kernel-app-blink: $(TOCK_ROOT_DIRECTORY)target/$(TARGET)/release/$(PLATFORM).bin erase-all
# Flash the kernel.
$(OPENOCD) -f interface/stlink.cfg -c "transport select hla_swd" -f target/stm32wlx.cfg \
-c "init; reset halt; program $< verify 0x08000000; reset; shutdown"
# Flash the blink app we temporarily are including in seeed studio board directory.
$(OPENOCD) -f interface/stlink.cfg -c "transport select hla_swd" -f target/stm32wlx.cfg -c \
"init; reset halt; program $(TOCK_ROOT_DIRECTORY)boards/seeed_studio_lora-E5-HF/blink.tbf verify 0x08020000; reset; shutdown"

Revert/remove this.

Comment on lines +136 to +137
// kernel::ipc::DRIVER_NUM => f(Some(&self.ipc)),
// capsules_core::gpio::DRIVER_NUM => f(Some(self.gpio)),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// kernel::ipc::DRIVER_NUM => f(Some(&self.ipc)),
// capsules_core::gpio::DRIVER_NUM => f(Some(self.gpio)),

Revert/remove this.

Comment on lines +339 to +344
let uart_device = static_init!(
capsules_core::virtualizers::virtual_uart::UartDevice<'static>,
capsules_core::virtualizers::virtual_uart::UartDevice::new(uart_mux_2, true)
);
uart_device.setup();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This uart_device is for sdi12 right?

I think the variable name should have something about sdi12 in it? Maybe sdi12_uart_device?

Comment on lines +498 to +499
//TODO: figure out why there needs to be a uart_device for the uart to transmit
// but the peripheral needs to be set to the transmit client for that to work
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this todo still pending?

Comment on lines +519 to +520
unsafe { &mut *addr_of_mut!(SDI12_TX_BUF) },
unsafe { &mut *addr_of_mut!(SDI12_RX_BUF) },
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We don't want to do this here. We should be able to do this without unsafe and just pass the slice/buffer.

#![crate_name = "stm32wle5xx"]
#![crate_type = "rlib"]
#![no_std]
#![allow(clippy::doc_suspicious_footnotes)]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This shouldn't be needed. I think this is related to adding [^1] back above (which exposed a clippy bug).

@@ -0,0 +1,199 @@
// Licensed under the Apache License, Version 2.0 or the MIT License.
// SPDX-License-Identifier: Apache-2.0 OR MIT
// Copyright Tock Contributors 2022.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Copyright Tock Contributors 2022.
// Copyright Tock Contributors 2025.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If possible, can you please pull these changes into a separate and clearly labeled commit?

Something like: chips: stm32wle5xx: usart: int. handler fixes

This will be helpful since this is orthogonal to the other sdi12 changes and this should get upstreamed more urgently.


// Completion conditions:
// 1) buffer full, OR
// 2) SDI-12 line complete on '\n'
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we need to make sdi12 specific changes to the usart driver?

Comment thread kernel/src/hil/sdi12.rs
@@ -0,0 +1,45 @@
// Licensed under the Apache License, Version 2.0 or the MIT License.
// SPDX-License-Identifier: Apache-2.0 OR MIT
// Copyright Tock Contributors 2022.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Copyright Tock Contributors 2022.
// Copyright Tock Contributors 2025.

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.

7 participants