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 status byte feature #40 #41

Merged
merged 3 commits into from
Jul 5, 2024
Merged

Conversation

Andrei-Basarab
Copy link
Contributor

  • Added status_byte.rs module in registers
  • Added get_last_chip_status_byte() API
  • Chip Status Byte is read on every SPI transaction

unrelated:

  • updated write_strobe method name to write_cmd_strobe, this way it seems more intuitive

- Added status_byte module in registers
- Added get_last_chip_status_byte() API
- Chip Status Byte is read on every SPI transaction
@Andrei-Basarab
Copy link
Contributor Author

Andrei-Basarab commented Jul 4, 2024

@dsvensson , can you link this PR with the issue #40 ?

@dsvensson
Copy link
Owner

dsvensson commented Jul 4, 2024

Agree with the naming change. But I wonder if status should perhaps be a core::option::Option that's set to None on write_cmd_strobe as that invalidates the current status, and None also better represents the initial state.

@Andrei-Basarab
Copy link
Contributor Author

Good point, I agree that Option<StatusByte> might be a better type for status and a more generic approach for the initial data.
Regarding sending of write_cmd_strobe(), not all commands have the same effect, and we shall receive the last status before executing the command.
Still, perhaps for state freshness and the user's peace of mind, we can simplify the behavior of setting it to None.
I will try to implement it later today.

src/lib.rs Outdated
@@ -58,6 +58,11 @@ where
Ok(Cc1101(lowlevel::Cc1101::new(spi)?))
}

/// Last Chip Status Byte
pub fn get_last_chip_status_byte(&mut self) -> Option<StatusByte> {
Copy link
Owner

@dsvensson dsvensson Jul 5, 2024

Choose a reason for hiding this comment

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

Maybe the name could be trimmed to get_last_chip_status or get_chip_status as get_last_chip_status_byte kind of duplicates the return type without adding any meaning, and that it's the last is kind of assumed, but maybe keep last(?), and Option adds the meaning that it's maybe not available.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can try to settle at get_chip_status and hope that the return type and the doc comment clarify the full meaning of the function

@dsvensson dsvensson merged commit df70ff0 into dsvensson:main Jul 5, 2024
1 check passed
@Andrei-Basarab Andrei-Basarab deleted the status-byte branch July 5, 2024 22:18
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