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

Adding support for reporting service information #131

Merged
merged 6 commits into from
Jan 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ shared-bus = { version = "0.2.0-alpha.1", features = ["cortex-m"] }
usb-device = "0.2.6"
postcard = "0.5.1"
crc-any = { version = "2.3.5", default-features = false }
panic-persist = { git = "https://github.com/jamesmunns/panic-persist", branch = "master", features = ["custom-panic-handler"] }
Copy link
Member Author

Choose a reason for hiding this comment

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

James Munns said he would push the master branch to a new release later today, so we should wait and update when that gets pushed.

Copy link
Member

Choose a reason for hiding this comment

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

Yes. Let's wait.

Copy link
Member Author

Choose a reason for hiding this comment

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

Deferring to #136


[dependencies.minimq]
git = "https://github.com/quartiq/minimq"
Expand Down
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -232,3 +232,13 @@ Bootloader USB interface.
```
dfu-util -a 0 -s 0x08000000:leave --download booster.bin
```

# Generating Releases

When a release is ready, `develop` must be merged into `master`.

The corresponding merge commit is then tagged with the version in the form of `vX.Y.Z`, where X, Y,
and Z are the semantic version major, minor, and patch versions. The tag must be pushed using `git
push origin vX.Y.Z`. This will automatically trigger CI to generate the release.

After the tag is generated, `master` must be merged back into `develop`.
55 changes: 55 additions & 0 deletions build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
//! Booster NGFW build script
//!
//! # Copyright
//! Copyright (C) 2020 QUARTIQ GmbH - All Rights Reserved
//! Unauthorized usage, editing, or copying is strictly prohibited.
//! Proprietary and confidential.
//!
//! # Note
//! This build script is run immediately before the build is completed and is used to inject
//! environment variables into the build.
use std::process::Command;

fn main() {
// Inject the git revision into an environment variable for compilation.
let dirty_flag = if !Command::new("git")
.args(&["diff", "--quiet"])
.status()
.unwrap()
.success()
{
"-dirty"
} else {
""
};

let output = Command::new("git")
.args(&["rev-parse", "HEAD"])
.output()
.unwrap();
let revision = String::from_utf8(output.stdout).unwrap();
println!(
"cargo:rustc-env=GIT_REVISION={}{}",
revision.trim(),
dirty_flag
);

let output = Command::new("git")
.args(&["describe", "--tags"])
.output()
.unwrap();
let version = String::from_utf8(output.stdout).unwrap();
println!("cargo:rustc-env=VERSION={}", version.trim());

// Collect all of the enabled features and inject them as an environment variable.
let mut features: Vec<String> = Vec::new();
for (key, _) in std::env::vars() {
let strings: Vec<&str> = key.split("CARGO_FEATURE_").collect();
if strings.len() > 1 {
println!("{}", strings[1]);
features.push(String::from(strings[1]));
}
}

println!("cargo:rustc-env=ALL_FEATURES={}", features.join(", "));
}
6 changes: 5 additions & 1 deletion memory.x
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@ MEMORY
{
/* NOTE 1 K = 1 KiBi = 1024 bytes */
FLASH : ORIGIN = 0x08000000, LENGTH = 1M
RAM : ORIGIN = 0x20000000, LENGTH = 128K
RAM : ORIGIN = 0x20000000, LENGTH = 127K
PANDUMP : ORIGIN = 0x2001FC00, LENGTH = 1K
}

_panic_dump_start = ORIGIN(PANDUMP);
_panic_dump_end = ORIGIN(PANDUMP) + LENGTH(PANDUMP);

/* This is where the call stack will be allocated. */
/* The stack is of the full descending type. */
/* You may want to use this variable to locate the call stack and static
Expand Down
5 changes: 2 additions & 3 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
#[macro_use]
extern crate log;

use panic_persist as _;

use core::fmt::Write;

use enum_iterator::IntoEnumIterator;
Expand Down Expand Up @@ -477,9 +479,6 @@ const APP: () = {
c.schedule.usb(c.start).unwrap();
c.schedule.fans(c.start).unwrap();

// Clear the reset flags now that initialization has completed.
platform::clear_reset_flags();

init::LateResources {
// Note that these share a resource because they both exist on the same I2C bus.
main_bus: MainBus { fans, channels },
Expand Down
5 changes: 4 additions & 1 deletion src/platform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,15 @@ use embedded_hal::{blocking::delay::DelayUs, digital::v2::OutputPin};
pub const MAXIMUM_REFLECTED_POWER_DBM: f32 = 30.0;

#[panic_handler]
fn panic(_info: &core::panic::PanicInfo) -> ! {
fn panic(info: &core::panic::PanicInfo) -> ! {
cortex_m::interrupt::disable();

// Shutdown all of the RF channels.
shutdown_channels();

// Write panic info to RAM.
panic_persist::report_panic_info(info);

// Reset the device in `release` configuration.
#[cfg(not(debug_assertions))]
cortex_m::peripheral::SCB::sys_reset();
Expand Down
61 changes: 61 additions & 0 deletions src/serial_terminal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ enum Token {

#[regex(r"[a-zA-Z0-9]+")]
DeviceIdentifier,

#[token("service")]
ServiceInfo,
}

#[derive(PartialEq)]
Expand All @@ -74,6 +77,7 @@ pub enum Request {
ResetBootloader,
Help,
Read(Property),
ServiceInfo,
WriteIpAddress(Property, Ipv4Addr),
WriteIdentifier(String<consts::U32>),
}
Expand Down Expand Up @@ -152,6 +156,61 @@ impl SerialTerminal {
);
}

Request::ServiceInfo => {
Copy link
Member

Choose a reason for hiding this comment

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

Also print version, release/debug, features and commit hash (through build.rs I guess). That makes the dump much more useful.

Copy link
Member Author

Choose a reason for hiding this comment

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

Service command has now become much more comprehensive:

> service
Version             : v0.1.0-50-gf59fce4
Git revision        : f59fce4f44f3790adf26d6be25a5917cb32793af-dirty
Features            : UNSTABLE
Panic Info          : None
Watchdog Detected   : false

> 

Version indicates the output of git describe --tags, Git revisionappends the-dirtyflag to indicate if untracked changes are present, andFeatures` is a comma-separated list of all enabled features. Other values self-explanatory.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe just use built (dev-dependency) for that. Also makes the git parsing and command execution somewhat more robust.

Copy link
Member Author

Choose a reason for hiding this comment

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

@jordens I was poking around with built and discovered an issue with cargo that prevents us from using built along with stable cargo - built enables std-features in serde, which is then (incorrectly) propagated to our application, which results in the app build failing if we use built in the [build-dependencies] section.

Necessary cargo changes were merged recently, but we'd have to move to nightly cargo.

We can move over to nightly cargo, but that would not be super optimal. We could alternatively leave this minimal implementation and spawn an issue to update to built when cargo stabilizes the fix.

Copy link
Member

Choose a reason for hiding this comment

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

Ah. I wasn't aware that crate features were also unified between dependencies and build-dependencies. That sounds bad.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a bug that just recently got fixed, but is not quite yet on stable cargo

let mut msg: String<consts::U256> = String::new();
write!(&mut msg, "{:<20}: {}\n", "Version", env!("VERSION")).unwrap_or_else(
|_| {
msg = String::from("Version: too long");
},
);
self.write(msg.as_bytes());

msg.clear();
write!(
&mut msg,
"{:<20}: {}\n",
"Git revision",
env!("GIT_REVISION")
)
.unwrap_or_else(|_| {
msg = String::from("Git revision: too long");
});
self.write(msg.as_bytes());

msg.clear();
write!(&mut msg, "{:<20}: {}\n", "Features", env!("ALL_FEATURES"))
.unwrap_or_else(|_| {
msg = String::from("Features: too long");
});
self.write(msg.as_bytes());

msg.clear();
// Note(unwrap): The msg size is long enough to always contain the provided
// string.
write!(&mut msg, "{:<20}: ", "Panic Info").unwrap();
self.write(msg.as_bytes());
self.write(
panic_persist::get_panic_message_bytes().unwrap_or("None".as_bytes()),
);
self.write("\n".as_bytes());

msg.clear();
// Note(unwrap): The msg size is long enough to be sufficient for all possible
// formats.
write!(
&mut msg,
"{:<20}: {}\n",
"Watchdog Detected",
platform::watchdog_detected()
)
.unwrap();
self.write(msg.as_bytes());

// Reading the panic message above clears the panic message, so similarly, we
// should also clear the watchdog once read.
platform::clear_reset_flags();
}

Request::WriteIpAddress(prop, addr) => match prop {
Property::SelfAddress => self.settings.set_ip_address(addr),
Property::BrokerAddress => self.settings.set_broker(addr),
Expand Down Expand Up @@ -312,6 +371,7 @@ gateway, netmask]
netmask, gateway] and <IP> must be an IP address (e.g. 192.168.1.1)
* `write id <ID>` - Write the MQTT client ID of the device. <ID> must be 23 or less ASCII \
characters.
* `service` - Read the service information. Service infromation clears once read.
".as_bytes(),
);
}
Expand All @@ -335,6 +395,7 @@ characters.
Token::Reset => Request::Reset,
Token::Dfu => Request::ResetBootloader,
Token::Help => Request::Help,
Token::ServiceInfo => Request::ServiceInfo,
Token::Read => {
// Validate that there is one valid token following.
let property_token = lex.next().ok_or("Malformed command")?;
Expand Down